R4R: CheckTx AnteHandler for Ethereum Tx Messages (#505)

* Rename Ethereum tx message
* Use new tx decoder in the ethermint app
* Update ante handler to prevent spam/dos
* Update ethereum msg signing/verification logic
* Implement secp256k1 key types
* Remove pointer from To method
* Move sig check to after inartistic gas check
* Add comment on chainID parsing
* Updated validateIntrinsicGas godoc
* Implement Fee method on eth tx msg
* Add reference to spec for recoverEthSig
* Upgrade TM to v0.27.0

x/evm/types/codec.go

@@ -15,5 +15,5 @@ func init() {
 
 // Register concrete types and interfaces on the given codec.
 func RegisterCodec(cdc *codec.Codec) {
-	cdc.RegisterConcrete(MsgEthereumTx{}, "ethermint/MsgEthereumTx", nil)
+	cdc.RegisterConcrete(&EthereumTxMsg{}, "ethermint/MsgEthereumTx", nil)
 }

x/evm/types/journal.go

@@ -94,7 +94,7 @@ type (
 
 	nonceChange struct {
 		account *ethcmn.Address
-		prev    int64
+		prev    uint64
 	}
 
 	storageChange struct {

x/evm/types/msg.go

@@ -8,6 +8,7 @@ import (
 	"math/big"
 	"sync/atomic"
 
+	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/ethermint/types"
 
@@ -17,17 +18,22 @@ import (
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
-var _ sdk.Msg = MsgEthereumTx{}
+var (
+	_ sdk.Msg = EthereumTxMsg{}
+	_ sdk.Tx  = EthereumTxMsg{}
+)
+
+var big8 = big.NewInt(8)
 
 // message type and route constants
 const (
-	TypeMsgEthereumTx  = "ethereum_tx"
-	RouteMsgEthereumTx = "evm"
+	TypeEthereumTxMsg  = "ethereum_tx"
+	RouteEthereumTxMsg = "evm"
 )
 
-// MsgEthereumTx encapsulates an Ethereum transaction as an SDK message.
+// EthereumTxMsg encapsulates an Ethereum transaction as an SDK message.
 type (
-	MsgEthereumTx struct {
+	EthereumTxMsg struct {
 		Data TxData
 
 		// caches
@@ -63,28 +69,28 @@ type (
 	}
 )
 
-// NewMsgEthereumTx returns a reference to a new Ethereum transaction message.
-func NewMsgEthereumTx(
+// NewEthereumTxMsg returns a reference to a new Ethereum transaction message.
+func NewEthereumTxMsg(
 	nonce uint64, to ethcmn.Address, amount *big.Int,
 	gasLimit uint64, gasPrice *big.Int, payload []byte,
-) *MsgEthereumTx {
+) *EthereumTxMsg {
 
-	return newMsgEthereumTx(nonce, &to, amount, gasLimit, gasPrice, payload)
+	return newEthereumTxMsg(nonce, &to, amount, gasLimit, gasPrice, payload)
 }
 
-// NewMsgEthereumTxContract returns a reference to a new Ethereum transaction
+// NewEthereumTxMsgContract returns a reference to a new Ethereum transaction
 // message designated for contract creation.
-func NewMsgEthereumTxContract(
+func NewEthereumTxMsgContract(
 	nonce uint64, amount *big.Int, gasLimit uint64, gasPrice *big.Int, payload []byte,
-) *MsgEthereumTx {
+) *EthereumTxMsg {
 
-	return newMsgEthereumTx(nonce, nil, amount, gasLimit, gasPrice, payload)
+	return newEthereumTxMsg(nonce, nil, amount, gasLimit, gasPrice, payload)
 }
 
-func newMsgEthereumTx(
+func newEthereumTxMsg(
 	nonce uint64, to *ethcmn.Address, amount *big.Int,
 	gasLimit uint64, gasPrice *big.Int, payload []byte,
-) *MsgEthereumTx {
+) *EthereumTxMsg {
 
 	if len(payload) > 0 {
 		payload = ethcmn.CopyBytes(payload)
@@ -109,18 +115,18 @@ func newMsgEthereumTx(
 		txData.Price.Set(gasPrice)
 	}
 
-	return &MsgEthereumTx{Data: txData}
+	return &EthereumTxMsg{Data: txData}
 }
 
-// Route returns the route value of an MsgEthereumTx.
-func (msg MsgEthereumTx) Route() string { return RouteMsgEthereumTx }
+// Route returns the route value of an EthereumTxMsg.
+func (msg EthereumTxMsg) Route() string { return RouteEthereumTxMsg }
 
-// Type returns the type value of an MsgEthereumTx.
-func (msg MsgEthereumTx) Type() string { return TypeMsgEthereumTx }
+// Type returns the type value of an EthereumTxMsg.
+func (msg EthereumTxMsg) Type() string { return TypeEthereumTxMsg }
 
 // ValidateBasic implements the sdk.Msg interface. It performs basic validation
 // checks of a Transaction. If returns an sdk.Error if validation fails.
-func (msg MsgEthereumTx) ValidateBasic() sdk.Error {
+func (msg EthereumTxMsg) ValidateBasic() sdk.Error {
 	if msg.Data.Price.Sign() != 1 {
 		return types.ErrInvalidValue("price must be positive")
 	}
@@ -132,12 +138,27 @@ func (msg MsgEthereumTx) ValidateBasic() sdk.Error {
 	return nil
 }
 
+// To returns the recipient address of the transaction. It returns nil if the
+// transaction is a contract creation.
+func (msg EthereumTxMsg) To() *ethcmn.Address {
+	if msg.Data.Recipient == nil {
+		return nil
+	}
+
+	return msg.Data.Recipient
+}
+
+// GetMsgs returns a single EthereumTxMsg as an sdk.Msg.
+func (msg EthereumTxMsg) GetMsgs() []sdk.Msg {
+	return []sdk.Msg{msg}
+}
+
 // GetSigners returns the expected signers for an Ethereum transaction message.
 // For such a message, there should exist only a single 'signer'.
 //
 // NOTE: This method cannot be used as a chain ID is needed to recover the signer
 // from the signature. Use 'VerifySig' instead.
-func (msg MsgEthereumTx) GetSigners() []sdk.AccAddress {
+func (msg EthereumTxMsg) GetSigners() []sdk.AccAddress {
 	panic("must use 'VerifySig' with a chain ID to get the signer")
 }
 
@@ -146,13 +167,13 @@ func (msg MsgEthereumTx) GetSigners() []sdk.AccAddress {
 //
 // NOTE: This method cannot be used as a chain ID is needed to create valid bytes
 // to sign over. Use 'RLPSignBytes' instead.
-func (msg MsgEthereumTx) GetSignBytes() []byte {
+func (msg EthereumTxMsg) GetSignBytes() []byte {
 	panic("must use 'RLPSignBytes' with a chain ID to get the valid bytes to sign")
 }
 
 // RLPSignBytes returns the RLP hash of an Ethereum transaction message with a
 // given chainID used for signing.
-func (msg MsgEthereumTx) RLPSignBytes(chainID *big.Int) ethcmn.Hash {
+func (msg EthereumTxMsg) RLPSignBytes(chainID *big.Int) ethcmn.Hash {
 	return rlpHash([]interface{}{
 		msg.Data.AccountNonce,
 		msg.Data.Price,
@@ -165,12 +186,12 @@ func (msg MsgEthereumTx) RLPSignBytes(chainID *big.Int) ethcmn.Hash {
 }
 
 // EncodeRLP implements the rlp.Encoder interface.
-func (msg *MsgEthereumTx) EncodeRLP(w io.Writer) error {
+func (msg *EthereumTxMsg) EncodeRLP(w io.Writer) error {
 	return rlp.Encode(w, &msg.Data)
 }
 
 // DecodeRLP implements the rlp.Decoder interface.
-func (msg *MsgEthereumTx) DecodeRLP(s *rlp.Stream) error {
+func (msg *EthereumTxMsg) DecodeRLP(s *rlp.Stream) error {
 	_, size, _ := s.Kind()
 
 	err := s.Decode(&msg.Data)
@@ -182,7 +203,7 @@ func (msg *MsgEthereumTx) DecodeRLP(s *rlp.Stream) error {
 }
 
 // Hash hashes the RLP encoding of a transaction.
-func (msg *MsgEthereumTx) Hash() ethcmn.Hash {
+func (msg *EthereumTxMsg) Hash() ethcmn.Hash {
 	if hash := msg.hash.Load(); hash != nil {
 		return hash.(ethcmn.Hash)
 	}
@@ -197,7 +218,7 @@ func (msg *MsgEthereumTx) Hash() ethcmn.Hash {
 // takes a private key and chainID to sign an Ethereum transaction according to
 // EIP155 standard. It mutates the transaction as it populates the V, R, S
 // fields of the Transaction's Signature.
-func (msg *MsgEthereumTx) Sign(chainID *big.Int, priv *ecdsa.PrivateKey) {
+func (msg *EthereumTxMsg) Sign(chainID *big.Int, priv *ecdsa.PrivateKey) {
 	txHash := msg.RLPSignBytes(chainID)
 
 	sig, err := ethcrypto.Sign(txHash[:], priv)
@@ -230,7 +251,7 @@ func (msg *MsgEthereumTx) Sign(chainID *big.Int, priv *ecdsa.PrivateKey) {
 
 // VerifySig attempts to verify a Transaction's signature for a given chainID.
 // A derived address is returned upon success or an error if recovery fails.
-func (msg MsgEthereumTx) VerifySig(chainID *big.Int) (ethcmn.Address, error) {
+func (msg *EthereumTxMsg) VerifySig(chainID *big.Int) (ethcmn.Address, error) {
 	signer := ethtypes.NewEIP155Signer(chainID)
 
 	if sc := msg.from.Load(); sc != nil {
@@ -244,43 +265,92 @@ func (msg MsgEthereumTx) VerifySig(chainID *big.Int) (ethcmn.Address, error) {
 
 	// do not allow recovery for transactions with an unprotected chainID
 	if chainID.Sign() == 0 {
-		return ethcmn.Address{}, errors.New("invalid chainID")
+		return ethcmn.Address{}, errors.New("chainID cannot be zero")
 	}
 
-	txHash := msg.RLPSignBytes(chainID)
-	sig := recoverEthSig(msg.Data.R, msg.Data.S, msg.Data.V, chainID)
+	chainIDMul := new(big.Int).Mul(chainID, big.NewInt(2))
+	V := new(big.Int).Sub(msg.Data.V, chainIDMul)
+	V.Sub(V, big8)
 
-	pub, err := ethcrypto.Ecrecover(txHash[:], sig)
+	sigHash := msg.RLPSignBytes(chainID)
+	sender, err := recoverEthSig(msg.Data.R, msg.Data.S, V, sigHash)
 	if err != nil {
 		return ethcmn.Address{}, err
 	}
 
-	var addr ethcmn.Address
-	copy(addr[:], ethcrypto.Keccak256(pub[1:])[12:])
-
-	msg.from.Store(sigCache{signer: signer, from: addr})
-	return addr, nil
+	msg.from.Store(sigCache{signer: signer, from: sender})
+	return sender, nil
 }
 
-// recoverEthSig recovers a signature according to the Ethereum specification.
-func recoverEthSig(R, S, Vb, chainID *big.Int) []byte {
-	var v byte
+// Cost returns amount + gasprice * gaslimit.
+func (msg EthereumTxMsg) Cost() *big.Int {
+	total := msg.Fee()
+	total.Add(total, msg.Data.Amount)
+	return total
+}
 
+// Fee returns gasprice * gaslimit.
+func (msg EthereumTxMsg) Fee() *big.Int {
+	return new(big.Int).Mul(msg.Data.Price, new(big.Int).SetUint64(msg.Data.GasLimit))
+}
+
+// ----------------------------------------------------------------------------
+// Auxiliary
+
+// TxDecoder returns an sdk.TxDecoder that can decode both auth.StdTx and
+// EthereumTxMsg transactions.
+func TxDecoder(cdc *codec.Codec) sdk.TxDecoder {
+	return func(txBytes []byte) (sdk.Tx, sdk.Error) {
+		var tx sdk.Tx
+
+		if len(txBytes) == 0 {
+			return nil, sdk.ErrTxDecode("txBytes are empty")
+		}
+
+		err := cdc.UnmarshalBinaryLengthPrefixed(txBytes, &tx)
+		if err != nil {
+			fmt.Println(err.Error())
+			return nil, sdk.ErrTxDecode("failed to decode tx").TraceSDK(err.Error())
+		}
+
+		return tx, nil
+	}
+}
+
+// recoverEthSig recovers a signature according to the Ethereum specification and
+// returns the sender or an error.
+//
+// Ref: Ethereum Yellow Paper (BYZANTIUM VERSION 69351d5) Appendix F
+func recoverEthSig(R, S, Vb *big.Int, sigHash ethcmn.Hash) (ethcmn.Address, error) {
+	if Vb.BitLen() > 8 {
+		return ethcmn.Address{}, errors.New("invalid signature")
+	}
+
+	V := byte(Vb.Uint64() - 27)
+	if !ethcrypto.ValidateSignatureValues(V, R, S, true) {
+		return ethcmn.Address{}, errors.New("invalid signature")
+	}
+
+	// encode the signature in uncompressed format
 	r, s := R.Bytes(), S.Bytes()
 	sig := make([]byte, 65)
 
 	copy(sig[32-len(r):32], r)
 	copy(sig[64-len(s):64], s)
+	sig[64] = V
 
-	if chainID.Sign() == 0 {
-		v = byte(Vb.Uint64() - 27)
-	} else {
-		chainIDMul := new(big.Int).Mul(chainID, big.NewInt(2))
-		V := new(big.Int).Sub(Vb, chainIDMul)
-
-		v = byte(V.Uint64() - 35)
+	// recover the public key from the signature
+	pub, err := ethcrypto.Ecrecover(sigHash[:], sig)
+	if err != nil {
+		return ethcmn.Address{}, err
 	}
 
-	sig[64] = v
-	return sig
+	if len(pub) == 0 || pub[0] != 4 {
+		return ethcmn.Address{}, errors.New("invalid public key")
+	}
+
+	var addr ethcmn.Address
+	copy(addr[:], ethcrypto.Keccak256(pub[1:])[12:])
+
+	return addr, nil
 }

x/evm/types/msg_test.go

@@ -6,8 +6,8 @@ import (
 	"math/big"
 	"testing"
 
+	"github.com/cosmos/ethermint/crypto"
 	ethcmn "github.com/ethereum/go-ethereum/common"
-	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/stretchr/testify/require"
 )
@@ -15,17 +15,17 @@ import (
 func TestMsgEthereumTx(t *testing.T) {
 	addr := GenerateEthAddress()
 
-	msg1 := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	msg1 := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 	require.NotNil(t, msg1)
 	require.Equal(t, *msg1.Data.Recipient, addr)
 
-	msg2 := NewMsgEthereumTxContract(0, nil, 100000, nil, []byte("test"))
+	msg2 := NewEthereumTxMsgContract(0, nil, 100000, nil, []byte("test"))
 	require.NotNil(t, msg2)
 	require.Nil(t, msg2.Data.Recipient)
 
-	msg3 := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
-	require.Equal(t, msg3.Route(), RouteMsgEthereumTx)
-	require.Equal(t, msg3.Type(), TypeMsgEthereumTx)
+	msg3 := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
+	require.Equal(t, msg3.Route(), RouteEthereumTxMsg)
+	require.Equal(t, msg3.Type(), TypeEthereumTxMsg)
 	require.Panics(t, func() { msg3.GetSigners() })
 	require.Panics(t, func() { msg3.GetSignBytes() })
 }
@@ -46,7 +46,7 @@ func TestMsgEthereumTxValidation(t *testing.T) {
 	}
 
 	for i, tc := range testCases {
-		msg := NewMsgEthereumTx(tc.nonce, tc.to, tc.amount, tc.gasLimit, tc.gasPrice, tc.payload)
+		msg := NewEthereumTxMsg(tc.nonce, tc.to, tc.amount, tc.gasLimit, tc.gasPrice, tc.payload)
 
 		if tc.expectPass {
 			require.Nil(t, msg.ValidateBasic(), "test: %v", i)
@@ -60,14 +60,14 @@ func TestMsgEthereumTxRLPSignBytes(t *testing.T) {
 	addr := ethcmn.BytesToAddress([]byte("test_address"))
 	chainID := big.NewInt(3)
 
-	msg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	msg := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 	hash := msg.RLPSignBytes(chainID)
 	require.Equal(t, "5BD30E35AD27449390B14C91E6BCFDCAADF8FE44EF33680E3BC200FC0DC083C7", fmt.Sprintf("%X", hash))
 }
 
 func TestMsgEthereumTxRLPEncode(t *testing.T) {
 	addr := ethcmn.BytesToAddress([]byte("test_address"))
-	msg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	msg := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 
 	raw, err := rlp.EncodeToBytes(msg)
 	require.NoError(t, err)
@@ -75,11 +75,11 @@ func TestMsgEthereumTxRLPEncode(t *testing.T) {
 }
 
 func TestMsgEthereumTxRLPDecode(t *testing.T) {
-	var msg MsgEthereumTx
+	var msg EthereumTxMsg
 
 	raw := ethcmn.FromHex("E48080830186A0940000000000000000746573745F61646472657373808474657374808080")
 	addr := ethcmn.BytesToAddress([]byte("test_address"))
-	expectedMsg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	expectedMsg := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 
 	err := rlp.Decode(bytes.NewReader(raw), &msg)
 	require.NoError(t, err)
@@ -88,34 +88,46 @@ func TestMsgEthereumTxRLPDecode(t *testing.T) {
 
 func TestMsgEthereumTxHash(t *testing.T) {
 	addr := ethcmn.BytesToAddress([]byte("test_address"))
-	msg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	msg := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 
 	hash := msg.Hash()
 	require.Equal(t, "E2AA2E68E7586AE9700F1D3D643330866B6AC2B6CA4C804F7C85ECB11D0B0B29", fmt.Sprintf("%X", hash))
 }
 
 func TestMsgEthereumTxSig(t *testing.T) {
-	priv, _ := ethcrypto.GenerateKey()
-	addr := PrivKeyToEthAddress(priv)
-
-	msg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
 	chainID := big.NewInt(3)
 
-	msg.Sign(chainID, priv)
+	priv1, _ := crypto.GenerateKey()
+	priv2, _ := crypto.GenerateKey()
+	addr1 := ethcmn.BytesToAddress(priv1.PubKey().Address().Bytes())
+	addr2 := ethcmn.BytesToAddress(priv2.PubKey().Address().Bytes())
 
-	resultAddr, err := msg.VerifySig(chainID)
+	// require valid signature passes validation
+	msg := NewEthereumTxMsg(0, addr1, nil, 100000, nil, []byte("test"))
+	msg.Sign(chainID, priv1.ToECDSA())
+
+	signer, err := msg.VerifySig(chainID)
 	require.NoError(t, err)
-	require.Equal(t, addr, resultAddr)
+	require.Equal(t, addr1, signer)
+	require.NotEqual(t, addr2, signer)
+
+	// require invalid chain ID fail validation
+	msg = NewEthereumTxMsg(0, addr1, nil, 100000, nil, []byte("test"))
+	msg.Sign(chainID, priv1.ToECDSA())
+
+	signer, err = msg.VerifySig(big.NewInt(4))
+	require.Error(t, err)
+	require.Equal(t, ethcmn.Address{}, signer)
 }
 
 func TestMsgEthereumTxAmino(t *testing.T) {
 	addr := GenerateEthAddress()
-	msg := NewMsgEthereumTx(0, addr, nil, 100000, nil, []byte("test"))
+	msg := NewEthereumTxMsg(0, addr, nil, 100000, nil, []byte("test"))
 
 	raw, err := msgCodec.MarshalBinaryBare(msg)
 	require.NoError(t, err)
 
-	var msg2 MsgEthereumTx
+	var msg2 EthereumTxMsg
 
 	err = msgCodec.UnmarshalBinaryBare(raw, &msg2)
 	require.NoError(t, err)

x/evm/types/state_object.go

@@ -176,10 +176,10 @@ func (so *stateObject) SetNonce(nonce uint64) {
 		prev:    so.account.Sequence,
 	})
 
-	so.setNonce(int64(nonce))
+	so.setNonce(nonce)
 }
 
-func (so *stateObject) setNonce(nonce int64) {
+func (so *stateObject) setNonce(nonce uint64) {
 	so.account.Sequence = nonce
 }
 
@@ -249,7 +249,7 @@ func (so *stateObject) CodeHash() []byte {
 
 // Nonce returns the state object's current nonce (sequence number).
 func (so *stateObject) Nonce() uint64 {
-	return uint64(so.account.Sequence)
+	return so.account.Sequence
 }
 
 // Code returns the contract code associated with this object, if any.

x/evm/types/utils.go

@@ -1,9 +1,9 @@
 package types
 
 import (
-	"crypto/ecdsa"
 	"fmt"
 
+	"github.com/cosmos/ethermint/crypto"
 	ethcmn "github.com/ethereum/go-ethereum/common"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 	ethsha "github.com/ethereum/go-ethereum/crypto/sha3"
@@ -12,19 +12,14 @@ import (
 	"github.com/pkg/errors"
 )
 
-// PrivKeyToEthAddress generates an Ethereum address given an ECDSA private key.
-func PrivKeyToEthAddress(p *ecdsa.PrivateKey) ethcmn.Address {
-	return ethcrypto.PubkeyToAddress(p.PublicKey)
-}
-
 // GenerateAddress generates an Ethereum address.
 func GenerateEthAddress() ethcmn.Address {
-	priv, err := ethcrypto.GenerateKey()
+	priv, err := crypto.GenerateKey()
 	if err != nil {
 		panic(err)
 	}
 
-	return PrivKeyToEthAddress(priv)
+	return ethcrypto.PubkeyToAddress(priv.PublicKey)
 }
 
 // ValidateSigner attempts to validate a signer for a given slice of bytes over