diff --git a/handlers/ante.go b/handlers/ante.go index 7b3b8ed5..3a8be0d6 100644 --- a/handlers/ante.go +++ b/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 } diff --git a/handlers/ante_test.go b/handlers/ante_test.go index 37bb19db..1a5d3a42 100644 --- a/handlers/ante_test.go +++ b/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] - - tx.Data.AccountNonce = 12 + // Create transaction with wrong nonce 12 + tx := types.NewTransaction(12, types.TestAddr2, sdk.NewInt(50), 1000, sdk.NewInt(1000), []byte("test_bytes")) + 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") - require.Equal(t, sdk.CodeOK, res.Code, fmt.Sprintf("Result not OK on valid Tx: %s", res.Log)) + assert.False(t, abort, "Antehandler abort on valid tx") + 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) { - cdc := types.NewTestCodec() +// Test EmbeddedTx Antehandler +// ----------------------------------------------------------------------------------------------------------------------------------- + +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") -} -*/ \ No newline at end of file diff --git a/types/test_common.go b/types/test_common.go index 5c0664a4..d344ede4 100644 --- a/types/test_common.go +++ b/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, ) diff --git a/types/tx.go b/types/tx.go index d29c2f0c..9fcfe1fe 100644 --- a/types/tx.go +++ b/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 } diff --git a/types/tx_test.go b/types/tx_test.go index b7a3dcb5..f4a553ce 100644 --- a/types/tx_test.go +++ b/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(), )