Fixed sig issue, wrote embedded tests

handlers/ante.go

@@ -47,15 +47,8 @@ func AnteHandler(am auth.AccountMapper) sdk.AnteHandler {
 			return sdkCtx, sdk.ErrInternal(fmt.Sprintf("invalid transaction: %T", tx)).Result(), true
 		}
 
-		/*
-		fmt.Println("handler begin")
-		fmt.Println(am.GetAccount(sdkCtx, tx.))
-		fmt.Println("handler end")*/
-
 		newCtx = sdkCtx.WithGasMeter(sdk.NewGasMeter(gasLimit))
 
-
-
 		// AnteHandlers must have their own defer/recover in order for the
 		// BaseApp to know how much gas was used! This is because the GasMeter
 		// is created in the AnteHandler, but if it panics the context won't be
@@ -101,9 +94,6 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont
 	sdkCtx.GasMeter().ConsumeGas(verifySigCost, "ante verify")
 	addr, err := ethTx.VerifySig(chainID)
 
-	fmt.Println(addr)
-	fmt.Println(chainID)
-
 	if err != nil {
 		return sdkCtx, sdk.ErrUnauthorized("signature verification failed").Result(), true
 	}
@@ -143,7 +133,7 @@ func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk
 		signer := ethcmn.BytesToAddress(signerAddrs[i].Bytes())
 
 		signerAcc, err := validateSignature(sdkCtx, etx, signer, sig, am)
-		if err.Code() != sdk.CodeOK {
+		if err != nil {
 			return sdkCtx, err.Result(), true
 		}
 

handlers/ante_test.go

@@ -16,9 +16,13 @@ import (
 
 	ethcmn "github.com/ethereum/go-ethereum/common"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+// Test EthTx Antehandler
+// -----------------------------------------------------------------------------------------------------------------------------------
+
 func TestBadSig(t *testing.T) {
 	tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0]
 
@@ -32,7 +36,7 @@ func TestBadSig(t *testing.T) {
 
 	_, res, abort := handler(ctx, tx)
 
-	require.True(t, abort, "Antehandler did not abort")
+	assert.True(t, abort, "Antehandler did not abort")
 	require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log))
 
 }
@@ -50,15 +54,15 @@ func TestInsufficientGas(t *testing.T) {
 
 	_, res, abort := handler(ctx, tx)
 
-	require.True(t, abort, "Antehandler did not abort")
+	assert.True(t, abort, "Antehandler did not abort")
 	require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log))
 
 }
 
 func TestIncorrectNonce(t *testing.T) {
-	tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0]
+	// Create transaction with wrong nonce 12
-
+	tx := types.NewTransaction(12, types.TestAddr2, sdk.NewInt(50), 1000, sdk.NewInt(1000), []byte("test_bytes"))
-	tx.Data.AccountNonce = 12
+	tx.Sign(types.TestChainID, types.TestPrivKey1)
 
 	ms, key := SetupMultiStore()
 	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, log.NewNopLogger())
@@ -71,14 +75,9 @@ func TestIncorrectNonce(t *testing.T) {
 
 	handler := AnteHandler(accountMapper)
 
-	fmt.Println("start test")
-	fmt.Println(accountMapper.GetAccount(ctx, types.TestAddr1[:]))
-	fmt.Println(types.TestAddr1)
-	fmt.Println("end test")
-
 	_, res, abort := handler(ctx, tx)
 
-	require.True(t, abort, "Antehandler did not abort")
+	assert.True(t, abort, "Antehandler did not abort")
 	require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log))
 
 }
@@ -90,51 +89,178 @@ func TestValidTx(t *testing.T) {
 	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
 
 	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+
+	// Set account in accountMapper
+	acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	accountMapper.SetAccount(ctx, acc)
+
 	handler := AnteHandler(accountMapper)
 
 	_, res, abort := handler(ctx, tx)
 
-	require.False(t, abort, "Antehandler abort on valid tx")
+	assert.False(t, abort, "Antehandler abort on valid tx")
-	require.Equal(t, sdk.CodeOK, res.Code, fmt.Sprintf("Result not OK on valid Tx: %s", res.Log))
+	require.True(t, res.IsOK(), fmt.Sprintf("Result not OK on valid Tx: %s", res.Log))
 
+	// Ensure account state updated correctly
+	seq, _ := accountMapper.GetSequence(ctx, types.TestAddr1[:])
+	require.Equal(t, int64(1), seq, "AccountNonce did not increment correctly")
 }
 
-func TestValidEmbeddedTx(t *testing.T) {
+// Test EmbeddedTx Antehandler
-	cdc := types.NewTestCodec()
+// -----------------------------------------------------------------------------------------------------------------------------------
+
+func TestInvalidSigEmbeddedTx(t *testing.T) {
 	// Create msg to be embedded
 	msgs := []sdk.Msg{stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)})}
 
-	tx := types.NewTestSDKTxs(cdc, types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1}, []int64{0}, []int64{0}, types.NewStdFee())[0]
+	// Create transaction with incorrect signer
+	tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey2},
+		 []int64{0}, []int64{0}, types.NewStdFee())
 
 	ms, key := SetupMultiStore()
 	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
 
 	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+
+	// Set account in accountMapper
+	acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	accountMapper.SetAccount(ctx, acc)
+
+	handler := AnteHandler(accountMapper)
+
+	_, res, abort := handler(ctx, tx)
+
+	assert.True(t, abort, "Antehandler did not abort on invalid embedded tx")
+	require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx")
+}
+
+func TestInvalidMultiMsgEmbeddedTx(t *testing.T) {
+	// Create msg to be embedded
+	msgs := []sdk.Msg{
+		stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+		stake.NewMsgDelegate(types.TestAddr2[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+	}
+
+	// Create transaction with only one signer
+	tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1},
+		[]int64{0}, []int64{0}, types.NewStdFee())
+
+	ms, key := SetupMultiStore()
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
+
+	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+
+	// Set account in accountMapper
+	acc1 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	accountMapper.SetAccount(ctx, acc1)
+	acc2 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	accountMapper.SetAccount(ctx, acc2)
+
+	handler := AnteHandler(accountMapper)
+
+	_, res, abort := handler(ctx, tx)
+
+	assert.True(t, abort, "Antehandler did not abort on invalid embedded tx")
+	require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx")
+}
+
+func TestInvalidAccountNumberEmbeddedTx(t *testing.T) {
+	// Create msg to be embedded
+	msgs := []sdk.Msg{
+		stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+	}
+
+	// Create transaction with wrong account number
+	tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1},
+		[]int64{3}, []int64{0}, types.NewStdFee())
+
+	ms, key := SetupMultiStore()
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
+
+	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+
+	// Set account in accountMapper
+	acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	acc.SetAccountNumber(4)
+	accountMapper.SetAccount(ctx, acc)
+
+	handler := AnteHandler(accountMapper)
+
+	_, res, abort := handler(ctx, tx)
+
+	assert.True(t, abort, "Antehandler did not abort on invalid embedded tx")
+	require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx")
+}
+
+func TestInvalidSequenceEmbeddedTx(t *testing.T) {
+	// Create msg to be embedded
+	msgs := []sdk.Msg{
+		stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+	}
+
+	// Create transaction with wrong account number
+	tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1},
+		[]int64{4}, []int64{2}, types.NewStdFee())
+
+	ms, key := SetupMultiStore()
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
+
+	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+
+	// Set account in accountMapper
+	acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	acc.SetAccountNumber(4)
+	acc.SetSequence(3)
+	accountMapper.SetAccount(ctx, acc)
+
+	handler := AnteHandler(accountMapper)
+
+	_, res, abort := handler(ctx, tx)
+
+	assert.True(t, abort, "Antehandler did not abort on invalid embedded tx")
+	require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx")
+}
+
+func TestValidEmbeddedTx(t *testing.T) {
+	// Create msg to be embedded
+	msgs := []sdk.Msg{
+		stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+		stake.NewMsgDelegate(types.TestAddr2[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}),
+	}
+
+	tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1, types.TestPrivKey2},
+		 []int64{4, 5}, []int64{3, 1}, types.NewStdFee())
+
+	ms, key := SetupMultiStore()
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
+
+	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
+	
+	// Set account in accountMapper
+	acc1 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:])
+	acc1.SetAccountNumber(4)
+	acc1.SetSequence(3)
+	accountMapper.SetAccount(ctx, acc1)
+	acc2 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr2[:])
+	acc2.SetAccountNumber(5)
+	acc2.SetSequence(1)
+	accountMapper.SetAccount(ctx, acc2)
+
 	handler := AnteHandler(accountMapper)
 
 	_, res, abort := handler(ctx, tx)
 
 	require.False(t, abort, "Antehandler abort on valid embedded tx")
-	require.Equal(t, sdk.CodeOK, res.Code, fmt.Sprintf("Result not OK on valid Tx: %s", res.Log))
+	require.True(t, res.IsOK(), fmt.Sprintf("Result not OK on valid Tx: %s", res.Log))
+
+	// Ensure account state updated correctly
+	seq1, _ := accountMapper.GetSequence(ctx, types.TestAddr1[:])
+	seq2, _ := accountMapper.GetSequence(ctx, types.TestAddr2[:])
+
+	assert.Equal(t, int64(4), seq1, "Sequence1 did not increment correctly")
+	assert.Equal(t, int64(2), seq2, "Sequence2 did not increment correctly")
+
 }
 
-/*
-func TestInvalidEmbeddedTx(t *testing.T) {
-	cdc := types.NewTestCodec()
-	// Create msg to be embedded
-	msgs := []sdk.Msg{stake.NewMsgCreateValidator(types.TestAddr1[:], nil, sdk.Coin{"steak", sdk.NewInt(50)}, stake.Description{})}
 
-	tx := types.NewTestSDKTxs(cdc, types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1}, []int64{0}, []int64{0}, types.NewStdFee())[0]
 
-	ms, key := SetupMultiStore()
-	ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil)
-
-	accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount)
-	handler := AnteHandler(accountMapper)
-
-	_, res, abort := handler(ctx, tx)
-
-	require.True(t, abort, "Antehandler did not abort on invalid embedded tx")
-	require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, "Result is OK on bad tx")
-}
-*/

types/test_common.go

@@ -42,7 +42,7 @@ func NewStdFee() auth.StdFee {
 	return auth.NewStdFee(5000, sdk.NewCoin("photon", 150))
 }
 
-func newTestEmbeddedTx(
+func NewTestEmbeddedTx(
 	chainID sdk.Int, msgs []sdk.Msg, pKeys []*ecdsa.PrivateKey,
 	accNums []int64, seqs []int64, fee auth.StdFee,
 ) sdk.Tx {
@@ -107,13 +107,13 @@ func NewTestSDKTxs(
 ) []Transaction {
 
 	txs := make([]Transaction, len(pKeys))
-	etx := newTestEmbeddedTx(chainID, msgs, pKeys, accNums, seqs, fee)
+	etx := NewTestEmbeddedTx(chainID, msgs, pKeys, accNums, seqs, fee)
 
 	for i, priv := range pKeys {
 		payload := codec.MustMarshalBinary(etx)
 
 		emintTx := NewTransaction(
-			uint64(i), TestSDKAddress, sdk.NewInt(10), 100,
+			uint64(i), TestSDKAddress, sdk.NewInt(10), 1000,
 			sdk.NewInt(100), payload,
 		)
 

types/tx.go

@@ -172,23 +172,28 @@ func (tx *Transaction) Sign(chainID sdk.Int, priv *ecdsa.PrivateKey) {
 }
 
 func (tx Transaction) VerifySig(chainID *big.Int) (ethcmn.Address, error) {
+	signer := ethtypes.NewEIP155Signer(chainID)
 	if sc := tx.from.Load(); sc != nil {
 		sigCache := sc.(sigCache)
 		// If the signer used to derive from in a previous
 		// call is not the same as used current, invalidate
 		// the cache.
-		if sigCache.signer.Equal(ethtypes.NewEIP155Signer(chainID)) {
+		if sigCache.signer.Equal(signer) {
 			return sigCache.from, nil
 		}
 	}
 
+	// Do not allow unprotected chainID
+	if chainID.Sign() == 0 {
+		return ethcmn.Address{}, errors.New("Cannot have 0 as ChainID")
+	}
 	
 	signBytes := rlpHash([]interface{}{
 		tx.Data.AccountNonce,
-		tx.Data.Price,
+		tx.Data.Price.BigInt(),
 		tx.Data.GasLimit,
 		tx.Data.Recipient,
-		tx.Data.Amount,
+		tx.Data.Amount.BigInt(),
 		tx.Data.Payload,
 		chainID, uint(0), uint(0),
 	})
@@ -200,9 +205,10 @@ func (tx Transaction) VerifySig(chainID *big.Int) (ethcmn.Address, error) {
 		return ethcmn.Address{}, err
 	}
 
-
 	var addr ethcmn.Address
 	copy(addr[:], ethcrypto.Keccak256(pub[1:])[12:])
+
+	tx.from.Store(sigCache{signer: signer, from: addr})
 	return addr, nil
 }
 

types/tx_test.go

@@ -100,7 +100,7 @@ func TestTransactionGetMsgs(t *testing.T) {
 	require.Equal(t, ethTxs[0], msgs[0])
 
 	expectedMsgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))}
-	etx := newTestEmbeddedTx(
+	etx := NewTestEmbeddedTx(
 		TestChainID, expectedMsgs, []*ecdsa.PrivateKey{TestPrivKey1},
 		[]int64{0}, []int64{0}, NewStdFee(),
 	)
@@ -112,7 +112,7 @@ func TestTransactionGetMsgs(t *testing.T) {
 
 func TestGetRequiredSigners(t *testing.T) {
 	msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))}
-	etx := newTestEmbeddedTx(
+	etx := NewTestEmbeddedTx(
 		TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1},
 		[]int64{0}, []int64{0}, NewStdFee(),
 	)
@@ -152,7 +152,7 @@ func TestTxDecoder(t *testing.T) {
 	require.Equal(t, emintTx, tx)
 
 	// create embedded transaction and encode
-	etx := newTestEmbeddedTx(
+	etx := NewTestEmbeddedTx(
 		TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1},
 		[]int64{0}, []int64{0}, NewStdFee(),
 	)