sign transactions using a signer interface instead of a private key (#328)

* sign transactions using a signer interface instead of a private key

* add mocks

* remove PrivateKey method from secp256k1.Keypair

Author: typestring

chains/evm/calls/evmclient/evm-client.go

@@ -2,16 +2,13 @@ package evmclient
 
 import (
 	"context"
-	"crypto/ecdsa"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"math/big"
 	"sync"
 	"time"
 
-	"github.com/ChainSafe/chainbridge-core/crypto/secp256k1"
-
 	"github.com/ethereum/go-ethereum"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -24,24 +21,31 @@ import (
 
 type EVMClient struct {
 	*ethclient.Client
-	kp         *secp256k1.Keypair
+	signer     Signer
 	gethClient *gethclient.Client
 	rpClient   *rpc.Client
 	nonce      *big.Int
 	nonceLock  sync.Mutex
 }
 
+type Signer interface {
+	CommonAddress() common.Address
+
+	// Sign calculates an ECDSA signature.
+	// The produced signature must be in the [R || S || V] format where V is 0 or 1.
+	Sign(digestHash []byte) ([]byte, error)
+}
+
 type CommonTransaction interface {
 	// Hash returns the transaction hash.
 	Hash() common.Hash
 
-	// RawWithSignature Returns signed transaction by provided private key
-	RawWithSignature(key *ecdsa.PrivateKey, domainID *big.Int) ([]byte, error)
+	// RawWithSignature Returns signed transaction by provided signer
+	RawWithSignature(signer Signer, domainID *big.Int) ([]byte, error)
 }
 
-// NewEVMClient creates a client for EVMChain with provided
-// private key.
-func NewEVMClient(url string, privateKey *ecdsa.PrivateKey) (*EVMClient, error) {
+// NewEVMClient creates a client for EVMChain with provided signer
+func NewEVMClient(url string, signer Signer) (*EVMClient, error) {
 	rpcClient, err := rpc.DialContext(context.TODO(), url)
 	if err != nil {
 		return nil, err
@@ -50,7 +54,7 @@ func NewEVMClient(url string, privateKey *ecdsa.PrivateKey) (*EVMClient, error)
 	c.Client = ethclient.NewClient(rpcClient)
 	c.gethClient = gethclient.New(rpcClient)
 	c.rpClient = rpcClient
-	c.kp = secp256k1.NewKeypair(*privateKey)
+	c.signer = signer
 	return c, nil
 }
 
@@ -160,17 +164,17 @@ func (c *EVMClient) PendingCallContract(ctx context.Context, callArgs map[string
 }
 
 func (c *EVMClient) From() common.Address {
-	return c.kp.CommonAddress()
+	return c.signer.CommonAddress()
 }
 
 func (c *EVMClient) SignAndSendTransaction(ctx context.Context, tx CommonTransaction) (common.Hash, error) {
 	id, err := c.ChainID(ctx)
 	if err != nil {
-		//panic(err)
+		// panic(err)
 		// Probably chain does not support chainID eg. CELO
 		id = nil
 	}
-	rawTx, err := tx.RawWithSignature(c.kp.PrivateKey(), id)
+	rawTx, err := tx.RawWithSignature(c.signer, id)
 	if err != nil {
 		return common.Hash{}, err
 	}
@@ -182,7 +186,7 @@ func (c *EVMClient) SignAndSendTransaction(ctx context.Context, tx CommonTransac
 }
 
 func (c *EVMClient) RelayerAddress() common.Address {
-	return c.kp.CommonAddress()
+	return c.signer.CommonAddress()
 }
 
 func (c *EVMClient) LockNonce() {
@@ -197,7 +201,7 @@ func (c *EVMClient) UnsafeNonce() (*big.Int, error) {
 	var err error
 	for i := 0; i <= 10; i++ {
 		if c.nonce == nil {
-			nonce, err := c.PendingNonceAt(context.Background(), c.kp.CommonAddress())
+			nonce, err := c.PendingNonceAt(context.Background(), c.signer.CommonAddress())
 			if err != nil {
 				time.Sleep(1 * time.Second)
 				continue

chains/evm/calls/evmtransaction/evm-tx.go

@@ -1,14 +1,14 @@
 package evmtransaction
 
 import (
-	"crypto/ecdsa"
-	"github.com/ChainSafe/chainbridge-core/chains/evm/calls/evmclient"
+	"context"
 	"math/big"
 
+	"github.com/ChainSafe/chainbridge-core/chains/evm/calls/evmclient"
+
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/crypto"
 )
 
 type TX struct {
@@ -19,12 +19,12 @@ type TX struct {
 // but return raw byte representation of transaction to be compatible and interchangeable between different go-ethereum forks
 // WithSignature returns a new transaction with the given signature.
 // This signature needs to be in the [R || S || V] format where V is 0 or 1.
-func (a *TX) RawWithSignature(key *ecdsa.PrivateKey, domainID *big.Int) ([]byte, error) {
-	opts, err := bind.NewKeyedTransactorWithChainID(key, domainID)
+func (a *TX) RawWithSignature(signer evmclient.Signer, domainID *big.Int) ([]byte, error) {
+	opts, err := newTransactorWithChainID(signer, domainID)
 	if err != nil {
 		return nil, err
 	}
-	tx, err := opts.Signer(crypto.PubkeyToAddress(key.PublicKey), a.tx)
+	tx, err := opts.Signer(signer.CommonAddress(), a.tx)
 	if err != nil {
 		return nil, err
 	}
@@ -76,3 +76,29 @@ func newTransaction(nonce uint64, to *common.Address, amount *big.Int, gasLimit
 func (a *TX) Hash() common.Hash {
 	return a.tx.Hash()
 }
+
+// newTransactorWithChainID is a utility method to easily create a transaction signer
+// for an evmclient.Signer.
+// Mostly copies bind.NewKeyedTransactorWithChainID but sings with the provided signer
+// instead of a privateKey
+func newTransactorWithChainID(s evmclient.Signer, chainID *big.Int) (*bind.TransactOpts, error) {
+	keyAddr := s.CommonAddress()
+	if chainID == nil {
+		return nil, bind.ErrNoChainID
+	}
+	signer := types.LatestSignerForChainID(chainID)
+	return &bind.TransactOpts{
+		From: keyAddr,
+		Signer: func(address common.Address, tx *types.Transaction) (*types.Transaction, error) {
+			if address != keyAddr {
+				return nil, bind.ErrNotAuthorized
+			}
+			signature, err := s.Sign(signer.Hash(tx).Bytes())
+			if err != nil {
+				return nil, err
+			}
+			return tx.WithSignature(signer, signature)
+		},
+		Context: context.Background(),
+	}, nil
+}

chains/evm/calls/evmtransaction/evm-tx_test.go

@@ -7,9 +7,10 @@ import (
 	evmgaspricer "github.com/ChainSafe/chainbridge-core/chains/evm/calls/evmgaspricer"
 	mock_evmgaspricer "github.com/ChainSafe/chainbridge-core/chains/evm/calls/evmgaspricer/mock"
 
-	"github.com/ChainSafe/chainbridge-core/keystore"
 	"github.com/ethereum/go-ethereum/core/types"
 
+	"github.com/ChainSafe/chainbridge-core/keystore"
+
 	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/golang/mock/gomock"
@@ -43,7 +44,7 @@ func (s *EVMTxTestSuite) TestNewTransactionWithStaticGasPricer() {
 	s.Nil(err)
 	tx, err := txFabric(1, &common.Address{}, big.NewInt(0), 10000, gp, []byte{})
 	s.Nil(err)
-	rawTx, err := tx.RawWithSignature(aliceKp.PrivateKey(), big.NewInt(420))
+	rawTx, err := tx.RawWithSignature(aliceKp, big.NewInt(420))
 	s.Nil(err)
 	txt := types.Transaction{}
 	err = txt.UnmarshalBinary(rawTx)
@@ -60,7 +61,7 @@ func (s *EVMTxTestSuite) TestNewTransactionWithLondonGasPricer() {
 	s.Nil(err)
 	tx, err := txFabric(1, &common.Address{}, big.NewInt(0), 10000, gp, []byte{})
 	s.Nil(err)
-	rawTx, err := tx.RawWithSignature(aliceKp.PrivateKey(), big.NewInt(420))
+	rawTx, err := tx.RawWithSignature(aliceKp, big.NewInt(420))
 	s.Nil(err)
 	txt := types.Transaction{}
 	err = txt.UnmarshalBinary(rawTx)

chains/evm/calls/transactor/itx/itx.go

@@ -6,7 +6,6 @@ import (
 	"math/big"
 
 	"github.com/ChainSafe/chainbridge-core/chains/evm/calls/transactor"
-	"github.com/ChainSafe/chainbridge-core/crypto/secp256k1"
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -56,17 +55,22 @@ type Forwarder interface {
 	ForwarderData(to *common.Address, data []byte, opts transactor.TransactOptions) ([]byte, error)
 }
 
+type Signer interface {
+	CommonAddress() common.Address
+	Sign(digestHash []byte) ([]byte, error)
+}
+
 type ITXTransactor struct {
 	forwarder   Forwarder
 	relayCaller RelayCaller
-	kp          *secp256k1.Keypair
+	signer      Signer
 }
 
-func NewITXTransactor(relayCaller RelayCaller, forwarder Forwarder, kp *secp256k1.Keypair) *ITXTransactor {
+func NewITXTransactor(relayCaller RelayCaller, forwarder Forwarder, signer Signer) *ITXTransactor {
 	return &ITXTransactor{
 		relayCaller: relayCaller,
 		forwarder:   forwarder,
-		kp:          kp,
+		signer:      signer,
 	}
 }
 
@@ -138,7 +142,7 @@ func (itx *ITXTransactor) signRelayTx(tx *RelayTx) (*SignedRelayTx, error) {
 	txID := crypto.Keccak256Hash(packed)
 	msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(txID), string(txID.Bytes()))
 	hash := crypto.Keccak256Hash([]byte(msg))
-	sig, err := crypto.Sign(hash.Bytes(), itx.kp.PrivateKey())
+	sig, err := itx.signer.Sign(hash.Bytes())
 	if err != nil {
 		return nil, err
 	}

chains/evm/calls/transactor/itx/minimalForwarder.go

@@ -7,7 +7,7 @@ import (
 
 	"github.com/ChainSafe/chainbridge-core/chains/evm/calls/contracts/forwarder"
 	"github.com/ChainSafe/chainbridge-core/chains/evm/calls/transactor"
-	"github.com/ChainSafe/chainbridge-core/crypto/secp256k1"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/math"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -27,7 +27,7 @@ type NonceStorer interface {
 }
 
 type MinimalForwarder struct {
-	kp                *secp256k1.Keypair
+	signer            Signer
 	nonce             *big.Int
 	nonceLock         sync.Mutex
 	chainID           *big.Int
@@ -36,10 +36,10 @@ type MinimalForwarder struct {
 }
 
 // NewMinimalForwarder creates an instance of MinimalForwarder
-func NewMinimalForwarder(chainID *big.Int, kp *secp256k1.Keypair, forwarderContract ForwarderContract, nonceStore NonceStorer) *MinimalForwarder {
+func NewMinimalForwarder(chainID *big.Int, signer Signer, forwarderContract ForwarderContract, nonceStore NonceStorer) *MinimalForwarder {
 	return &MinimalForwarder{
 		chainID:           chainID,
-		kp:                kp,
+		signer:            signer,
 		forwarderContract: forwarderContract,
 		nonceStore:        nonceStore,
 	}
@@ -73,8 +73,7 @@ func (c *MinimalForwarder) UnsafeNonce() (*big.Int, error) {
 		if err != nil {
 			return nil, err
 		}
-
-		from := common.HexToAddress(c.kp.Address())
+		from := c.signer.CommonAddress()
 		contractNonce, err := c.forwarderContract.GetNonce(from)
 		if err != nil {
 			return nil, err
@@ -110,7 +109,7 @@ func (c *MinimalForwarder) ChainId() *big.Int {
 
 // ForwarderData returns ABI packed and signed byte data for a forwarded transaction
 func (c *MinimalForwarder) ForwarderData(to *common.Address, data []byte, opts transactor.TransactOptions) ([]byte, error) {
-	from := c.kp.Address()
+	from := c.signer.CommonAddress().Hex()
 	forwarderHash, err := c.typedHash(
 		from,
 		to.String(),
@@ -124,7 +123,7 @@ func (c *MinimalForwarder) ForwarderData(to *common.Address, data []byte, opts t
 		return nil, err
 	}
 
-	sig, err := crypto.Sign(forwarderHash, c.kp.PrivateKey())
+	sig, err := c.signer.Sign(forwarderHash)
 	if err != nil {
 		return nil, err
 	}

chains/evm/calls/transactor/itx/mock/itx.go

@@ -1,7 +1,6 @@
 package deploy
 
 import (
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"math/big"
@@ -20,7 +19,6 @@ import (
 	"github.com/ChainSafe/chainbridge-core/chains/evm/cli/logger"
 	"github.com/ChainSafe/chainbridge-core/chains/evm/cli/utils"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
 )
@@ -170,9 +168,9 @@ func DeployCLI(cmd *cobra.Command, args []string, txFabric calls.TxFabric, gasPr
 	}
 
 	log.Debug().Msgf("url: %s gas limit: %v gas price: %v", url, gasLimit, gasPrice)
-	log.Debug().Msgf("SENDER Private key 0x%s", hex.EncodeToString(crypto.FromECDSA(senderKeyPair.PrivateKey())))
+	log.Debug().Msgf("SENDER Address %s", senderKeyPair.CommonAddress().Hex())
 
-	ethClient, err := evmclient.NewEVMClient(url, senderKeyPair.PrivateKey())
+	ethClient, err := evmclient.NewEVMClient(url, senderKeyPair)
 	if err != nil {
 		log.Error().Err(fmt.Errorf("ethereum client error: %v", err)).Msg("error initializing new EVM client")
 		return err

chains/evm/cli/deploy/deploy.go

@@ -18,8 +18,7 @@ func InitializeClient(
 	url string,
 	senderKeyPair *secp256k1.Keypair,
 ) (*evmclient.EVMClient, error) {
-	ethClient, err := evmclient.NewEVMClient(
-		url, senderKeyPair.PrivateKey())
+	ethClient, err := evmclient.NewEVMClient(url, senderKeyPair)
 	if err != nil {
 		log.Error().Err(fmt.Errorf("eth client initialization error: %v", err))
 		return nil, err