From e114ec508e2e7141e13a44878737b7115b6a40c6 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 13 Aug 2018 18:02:21 -0700 Subject: [PATCH 1/5] simplified sig verification, started handler tests --- Gopkg.lock | 128 +++------------------- handlers/ante.go | 9 +- handlers/ante_test.go | 70 ++++++++++++ handlers/test_common.go | 16 +++ types/test_common.go | 122 +++++++++++++++++++++ types/tx.go | 78 ++++++++++---- types/tx_test.go | 228 ++++++++-------------------------------- types/utils_test.go | 12 +-- 8 files changed, 329 insertions(+), 334 deletions(-) create mode 100644 handlers/ante_test.go create mode 100644 handlers/test_common.go create mode 100644 types/test_common.go diff --git a/Gopkg.lock b/Gopkg.lock index 4b8fd799c..f6b028f6f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -3,29 +3,22 @@ [[projects]] branch = "master" - digest = "1:fcdf62d2d7e43c2565d6f8707ab4eae54dac702ed4bafb194b85139f0508929f" name = "github.com/aristanetworks/goarista" packages = ["monotime"] - pruneopts = "T" revision = "b2d71c282dc706f4b4f6c15b65810e1202ecd53f" [[projects]] branch = "master" - digest = "1:d4d66abd43dbb9b5f5e6a176c5ed279c289f8db734904c047d95113a04aa2e60" name = "github.com/btcsuite/btcd" packages = ["btcec"] - pruneopts = "T" revision = "cf05f92c3f815bbd5091ed6c73eff51f7b1945e8" [[projects]] - digest = "1:d0d998526cfb68788229a31c16a557fdf1fbbb510654be6b3732c2758e06b533" name = "github.com/btcsuite/btcutil" packages = ["bech32"] - pruneopts = "T" revision = "d4cc87b860166d00d6b5b9e0d3b3d71d6088d4d4" [[projects]] - digest = "1:36773b598dec105de46a87978ae14e64c8d2c45aa556b8e0ddfc62d6abc7c47e" name = "github.com/cosmos/cosmos-sdk" packages = [ "baseapp", @@ -33,30 +26,24 @@ "types", "version", "wire", - "x/auth", + "x/auth" ] - pruneopts = "T" revision = "23e3d5ac12145c02fcb4b4767d7dfccad782aee5" version = "v0.23.1" [[projects]] - digest = "1:52f195ad0e20a92d8604c1ba3cd246c61644c03eaa454b5acd41be89841e0d10" name = "github.com/davecgh/go-spew" packages = ["spew"] - pruneopts = "T" revision = "346938d642f2ec3594ed81d874461961cd0faa76" version = "v1.1.0" [[projects]] branch = "master" - digest = "1:67d0b50be0549e610017cb91e0b0b745ec0cad7c613bc8e18ff2d1c1fc8825a7" name = "github.com/edsrzf/mmap-go" packages = ["."] - pruneopts = "T" revision = "0bce6a6887123b67a60366d2c9fe2dfb74289d2e" [[projects]] - digest = "1:3238a0c355a81640974751f7d3bab21bf91035165f75c2c457959425c0422a4b" name = "github.com/ethereum/go-ethereum" packages = [ "common", @@ -86,42 +73,34 @@ "params", "rlp", "rpc", - "trie", + "trie" ] - pruneopts = "T" revision = "dea1ce052a10cd7d401a5c04f83f371a06fe293c" version = "v1.8.11" [[projects]] - digest = "1:0b9c3ad6c948d57a379da9c4e1cdd989b1c73ddc5ec8673f52a9539ce60a109b" name = "github.com/go-kit/kit" packages = [ "log", "log/level", - "log/term", + "log/term" ] - pruneopts = "T" revision = "4dc7be5d2d12881735283bcab7352178e190fc71" version = "v0.6.0" [[projects]] - digest = "1:31a18dae27a29aa074515e43a443abfd2ba6deb6d69309d8d7ce789c45f34659" name = "github.com/go-logfmt/logfmt" packages = ["."] - pruneopts = "T" revision = "390ab7935ee28ec6b286364bba9b4dd6410cb3d5" version = "v0.3.0" [[projects]] - digest = "1:c4a2528ccbcabf90f9f3c464a5fc9e302d592861bbfd0b7135a7de8a943d0406" name = "github.com/go-stack/stack" packages = ["."] - pruneopts = "T" revision = "259ab82a6cad3992b4e21ff5cac294ccb06474bc" version = "v1.7.0" [[projects]] - digest = "1:da39f4a22829ca95e63566208e0ea42d6f055f41dff1b14fdab88d88f62df653" name = "github.com/gogo/protobuf" packages = [ "gogoproto", @@ -129,122 +108,96 @@ "proto", "protoc-gen-gogo/descriptor", "sortkeys", - "types", + "types" ] - pruneopts = "T" revision = "636bf0302bc95575d69441b25a2603156ffdddf1" version = "v1.1.1" [[projects]] - digest = "1:832e17df5ff8bbe0e0693d2fb46c5e53f96c662ee804049ce3ab6557df74e3ab" name = "github.com/golang/protobuf" packages = [ "proto", "ptypes", "ptypes/any", "ptypes/duration", - "ptypes/timestamp", + "ptypes/timestamp" ] - pruneopts = "T" revision = "b4deda0973fb4c70b50d226b1af49f3da59f5265" version = "v1.1.0" [[projects]] branch = "master" - digest = "1:6027b20c168728321bd99ad01f35118eded457b01c03e647a84833ab331f2f5b" name = "github.com/golang/snappy" packages = ["."] - pruneopts = "T" revision = "2e65f85255dbc3072edf28d6b5b8efc472979f5a" [[projects]] - digest = "1:cf296baa185baae04a9a7004efee8511d08e2f5f51d4cbe5375da89722d681db" name = "github.com/hashicorp/golang-lru" packages = [ ".", - "simplelru", + "simplelru" ] - pruneopts = "T" revision = "0fb14efe8c47ae851c0034ed7a448854d3d34cf3" [[projects]] - digest = "1:870d441fe217b8e689d7949fef6e43efbc787e50f200cb1e70dbca9204a1d6be" name = "github.com/inconshreveable/mousetrap" packages = ["."] - pruneopts = "T" revision = "76626ae9c91c4f2a10f34cad8ce83ea42c93bb75" version = "v1.0" [[projects]] branch = "master" - digest = "1:dc6b1a6801b3055e9bd3da4cd1e568606eb48118cc6f28e947783aa5d998ad74" name = "github.com/jmhodges/levigo" packages = ["."] - pruneopts = "T" revision = "c42d9e0ca023e2198120196f842701bb4c55d7b9" [[projects]] branch = "master" - digest = "1:a64e323dc06b73892e5bb5d040ced475c4645d456038333883f58934abbf6f72" name = "github.com/kr/logfmt" packages = ["."] - pruneopts = "T" revision = "b84e30acd515aadc4b783ad4ff83aff3299bdfe0" [[projects]] - digest = "1:40e195917a951a8bf867cd05de2a46aaf1806c50cf92eebf4c16f78cd196f747" name = "github.com/pkg/errors" packages = ["."] - pruneopts = "T" revision = "645ef00459ed84a119197bfb8d8205042c6df63d" version = "v0.8.0" [[projects]] - digest = "1:22aa691fe0213cb5c07d103f9effebcb7ad04bee45a0ce5fe5369d0ca2ec3a1f" name = "github.com/pmezard/go-difflib" packages = ["difflib"] - pruneopts = "T" revision = "792786c7400a136282c1664665ae0a8db921c6c2" version = "v1.0.0" [[projects]] - digest = "1:6cae6970d70fc5fe75bf83c48ee33e9c4c561a62d0b033254bee8dd5942b815a" name = "github.com/rs/cors" packages = ["."] - pruneopts = "T" revision = "3fb1b69b103a84de38a19c3c6ec073dd6caa4d3f" version = "v1.5.0" [[projects]] - digest = "1:8be8b3743fc9795ec21bbd3e0fc28ff6234018e1a269b0a7064184be95ac13e0" name = "github.com/spf13/cobra" packages = ["."] - pruneopts = "T" revision = "ef82de70bb3f60c65fb8eebacbb2d122ef517385" version = "v0.0.3" [[projects]] - digest = "1:6de2f73eb31e80d74f84ce1c861e4c0c8f00ca5fb41a25901f987e63a0647c28" name = "github.com/spf13/pflag" packages = ["."] - pruneopts = "T" revision = "583c0c0531f06d5278b7d917446061adc344b5cd" version = "v1.0.1" [[projects]] - digest = "1:e95496462101745805bd4e041a5b841e108c7cf761264d53648246308de2761e" name = "github.com/stretchr/testify" packages = [ "assert", - "require", + "require" ] - pruneopts = "T" revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" version = "v1.2.2" [[projects]] branch = "master" - digest = "1:7d44c4d11eb65cfdc78c76040f37ef305b16474c019c98a8a7cf188fece2d574" name = "github.com/syndtr/goleveldb" packages = [ "leveldb", @@ -258,41 +211,33 @@ "leveldb/opt", "leveldb/storage", "leveldb/table", - "leveldb/util", + "leveldb/util" ] - pruneopts = "T" revision = "c4c61651e9e37fa117f53c5a906d3b63090d8445" [[projects]] branch = "master" - digest = "1:2b15c0442dc80b581ce7028b2e43029d2f3f985da43cb1d55f7bcdeca785bda0" name = "github.com/tendermint/ed25519" packages = [ ".", "edwards25519", - "extra25519", + "extra25519" ] - pruneopts = "T" revision = "d8387025d2b9d158cf4efb07e7ebf814bcce2057" [[projects]] - digest = "1:0e2addab3f64ece97ca434b2bf2d4e8cb54a4509904a03be8c81da3fc2ddb245" name = "github.com/tendermint/go-amino" packages = ["."] - pruneopts = "T" revision = "2106ca61d91029c931fd54968c2bb02dc96b1412" version = "0.10.1" [[projects]] - digest = "1:bf042d2f7d1252b9dcae8e694e2f0a9b5294cb357c086fd86dc540d2f32c9fdf" name = "github.com/tendermint/iavl" packages = ["."] - pruneopts = "T" revision = "35f66e53d9b01e83b30de68b931f54b2477a94c9" version = "v0.9.2" [[projects]] - digest = "1:9f6704ae2aedbadf616e5850375c504909d46b6ea57d4679de2b7cbc715f08e1" name = "github.com/tendermint/tendermint" packages = [ "abci/server", @@ -309,22 +254,18 @@ "libs/log", "libs/pubsub", "libs/pubsub/query", - "types", + "types" ] - pruneopts = "T" revision = "d542d2c3945116697f60451e6a407082c41c3cc9" version = "v0.22.8" [[projects]] branch = "master" - digest = "1:2cbe8758697d867fcebf73bcc69dff8e8abaa7fd65e5704e0744e522ccff4e6a" name = "golang.org/x/crypto" packages = ["ripemd160"] - pruneopts = "T" revision = "f027049dab0ad238e394a753dba2d14753473a04" [[projects]] - digest = "1:5fdc7adede42f80d6201258355d478d856778e21d735f14972abd8ff793fdbf7" name = "golang.org/x/net" packages = [ "context", @@ -334,13 +275,11 @@ "idna", "internal/timeseries", "trace", - "websocket", + "websocket" ] - pruneopts = "T" revision = "292b43bbf7cb8d35ddf40f8d5100ef3837cced3f" [[projects]] - digest = "1:6164911cb5e94e8d8d5131d646613ff82c14f5a8ce869de2f6d80d9889df8c5a" name = "golang.org/x/text" packages = [ "collate", @@ -356,21 +295,17 @@ "unicode/bidi", "unicode/cldr", "unicode/norm", - "unicode/rangetable", + "unicode/rangetable" ] - pruneopts = "T" revision = "f21a4dfb5e38f5895301dc265a8def02365cc3d0" version = "v0.3.0" [[projects]] - digest = "1:8cfa91d1b7f6b66fa9b1a738a4bc1325837b861e63fb9a2919931d68871bb770" name = "google.golang.org/genproto" packages = ["googleapis/rpc/status"] - pruneopts = "T" revision = "7fd901a49ba6a7f87732eb344f6e3c5b19d1b200" [[projects]] - digest = "1:adafc60b1d4688759f3fc8f9089e71dd17abd123f4729de6b913bf08c9143770" name = "google.golang.org/grpc" packages = [ ".", @@ -397,67 +332,32 @@ "stats", "status", "tap", - "transport", + "transport" ] - pruneopts = "T" revision = "168a6198bcb0ef175f7dacec0b8691fc141dc9b8" version = "v1.13.0" [[projects]] - digest = "1:3ccd10c863188cfe0d936fcfe6a055c95362e43af8e7039e33baade846928e74" name = "gopkg.in/fatih/set.v0" packages = ["."] - pruneopts = "T" revision = "57907de300222151a123d29255ed17f5ed43fad3" version = "v0.1.0" [[projects]] branch = "v2" - digest = "1:dae137be246befa42ce4b48c0feff2c5796b8a5027139a283f31a21173744410" name = "gopkg.in/karalabe/cookiejar.v2" packages = ["collections/prque"] - pruneopts = "T" revision = "8dcd6a7f4951f6ff3ee9cbb919a06d8925822e57" [[projects]] branch = "v2" - digest = "1:3d3f9391ab615be8655ae0d686a1564f3fec413979bb1aaf018bac1ec1bb1cc7" name = "gopkg.in/natefinch/npipe.v2" packages = ["."] - pruneopts = "T" revision = "c1b8fa8bdccecb0b8db834ee0b92fdbcfa606dd6" [solve-meta] analyzer-name = "dep" analyzer-version = 1 - input-imports = [ - "github.com/cosmos/cosmos-sdk/baseapp", - "github.com/cosmos/cosmos-sdk/store", - "github.com/cosmos/cosmos-sdk/types", - "github.com/cosmos/cosmos-sdk/wire", - "github.com/cosmos/cosmos-sdk/x/auth", - "github.com/ethereum/go-ethereum/common", - "github.com/ethereum/go-ethereum/common/math", - "github.com/ethereum/go-ethereum/consensus", - "github.com/ethereum/go-ethereum/consensus/ethash", - "github.com/ethereum/go-ethereum/consensus/misc", - "github.com/ethereum/go-ethereum/core", - "github.com/ethereum/go-ethereum/core/state", - "github.com/ethereum/go-ethereum/core/types", - "github.com/ethereum/go-ethereum/core/vm", - "github.com/ethereum/go-ethereum/crypto", - "github.com/ethereum/go-ethereum/crypto/sha3", - "github.com/ethereum/go-ethereum/ethdb", - "github.com/ethereum/go-ethereum/params", - "github.com/ethereum/go-ethereum/rlp", - "github.com/ethereum/go-ethereum/rpc", - "github.com/ethereum/go-ethereum/trie", - "github.com/hashicorp/golang-lru", - "github.com/pkg/errors", - "github.com/stretchr/testify/require", - "github.com/tendermint/tendermint/libs/common", - "github.com/tendermint/tendermint/libs/db", - "github.com/tendermint/tendermint/libs/log", - ] + inputs-digest = "560c44b5e8f495ae409bc13b0e32cf36e3281f0d02b68b15e987faa2cda1e91c" solver-name = "gps-cdcl" solver-version = 1 diff --git a/handlers/ante.go b/handlers/ante.go index 09ad13c1c..a62be2142 100644 --- a/handlers/ante.go +++ b/handlers/ante.go @@ -9,8 +9,6 @@ import ( "github.com/cosmos/ethermint/types" ethcmn "github.com/ethereum/go-ethereum/common" - - ethtypes "github.com/ethereum/go-ethereum/core/types" ) const ( @@ -93,10 +91,9 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont } // validate signature - gethTx := ethTx.ConvertTx(chainID) - signer := ethtypes.NewEIP155Signer(chainID) + sdkCtx.GasMeter().ConsumeGas(verifySigCost, "ante verify") + _, err := ethTx.VerifySig(chainID) - _, err := signer.Sender(&gethTx) if err != nil { return sdkCtx, sdk.ErrUnauthorized("signature verification failed").Result(), true } @@ -125,7 +122,7 @@ func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk signerAcc, err := validateSignature(sdkCtx, etx, signer, sig, am) if err.Code() != sdk.CodeOK { - return sdkCtx, err.Result(), false + return sdkCtx, err.Result(), true } // TODO: Fees! diff --git a/handlers/ante_test.go b/handlers/ante_test.go new file mode 100644 index 000000000..2cfc62850 --- /dev/null +++ b/handlers/ante_test.go @@ -0,0 +1,70 @@ +package handlers + +import ( + "testing" + "math/big" + "crypto/ecdsa" + + "github.com/cosmos/ethermint/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth" + abci "github.com/tendermint/tendermint/abci/types" + + ethcmn "github.com/ethereum/go-ethereum/common" + + "github.com/stretchr/testify/require" +) + +func TestBadSig(t *testing.T) { + tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + + tx.Data.Signature = types.NewEthSignature(new(big.Int), new(big.Int), new(big.Int)) + + 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") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx") + +} + +func TestInsufficientGas(t *testing.T) { + tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + + tx.Data.GasLimit = 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") + require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, "Result is OK on bad tx") + +} + +func TestWrongNonce(t *testing.T) { + tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + + tx.Data.AccountNonce = 12 + + 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") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx") + +} \ No newline at end of file diff --git a/handlers/test_common.go b/handlers/test_common.go new file mode 100644 index 000000000..901f01e9c --- /dev/null +++ b/handlers/test_common.go @@ -0,0 +1,16 @@ +package handlers + +import ( + "github.com/cosmos/cosmos-sdk/store" + sdk "github.com/cosmos/cosmos-sdk/types" + dbm "github.com/tendermint/tendermint/libs/db" +) + +func SetupMultiStore() (sdk.MultiStore, *sdk.KVStoreKey) { + db := dbm.NewMemDB() + capKey := sdk.NewKVStoreKey("capkey") + ms := store.NewCommitMultiStore(db) + ms.MountStoreWithDB(capKey, sdk.StoreTypeIAVL, db) + ms.LoadLatestVersion() + return ms, capKey +} diff --git a/types/test_common.go b/types/test_common.go new file mode 100644 index 000000000..ab2aa1119 --- /dev/null +++ b/types/test_common.go @@ -0,0 +1,122 @@ +package types + +import ( + "crypto/ecdsa" + "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/wire" + "github.com/cosmos/cosmos-sdk/x/auth" + + ethcrypto "github.com/ethereum/go-ethereum/crypto" + ethtypes "github.com/ethereum/go-ethereum/core/types" + ethcmn "github.com/ethereum/go-ethereum/common" +) + +var ( + TestChainID = sdk.NewInt(3) + + TestPrivKey1, _ = ethcrypto.GenerateKey() + TestPrivKey2, _ = ethcrypto.GenerateKey() + + TestAddr1 = PrivKeyToEthAddress(TestPrivKey1) + TestAddr2 = PrivKeyToEthAddress(TestPrivKey2) + + TestSDKAddress = GenerateEthAddress() +) + +func NewTestCodec() *wire.Codec { + codec := wire.NewCodec() + + RegisterWire(codec) + codec.RegisterConcrete(&sdk.TestMsg{}, "test/TestMsg", nil) + + return codec +} + +func newStdFee() auth.StdFee { + return auth.NewStdFee(5000, sdk.NewCoin("photon", 150)) +} + +func newTestEmbeddedTx( + chainID sdk.Int, msgs []sdk.Msg, pKeys []*ecdsa.PrivateKey, + accNums []int64, seqs []int64, fee auth.StdFee, +) sdk.Tx { + + sigs := make([][]byte, len(pKeys)) + + for i, priv := range pKeys { + signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, newStdFee()} + + signBytes, err := signEtx.Bytes() + if err != nil { + panic(err) + } + + sig, err := ethcrypto.Sign(signBytes, priv) + if err != nil { + panic(err) + } + + sigs[i] = sig + } + + return EmbeddedTx{msgs, fee, sigs} +} + +func NewTestGethTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Address) []ethtypes.Transaction { + txs := make([]ethtypes.Transaction, len(pKeys)) + + for i, priv := range pKeys { + ethTx := ethtypes.NewTransaction( + uint64(i), addrs[i], big.NewInt(10), 100, big.NewInt(100), nil, + ) + + signer := ethtypes.NewEIP155Signer(chainID.BigInt()) + ethTx, _ = ethtypes.SignTx(ethTx, signer, priv) + + txs[i] = *ethTx + } + + return txs +} + +func NewTestEthTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Address) []Transaction { + txs := make([]Transaction, len(pKeys)) + + for i, priv := range pKeys { + emintTx := NewTransaction( + uint64(i), addrs[i], sdk.NewInt(10), 100, sdk.NewInt(100), nil, + ) + + emintTx.Sign(chainID, priv) + + txs[i] = emintTx + } + + return txs +} + +func NewTestSDKTxs( + codec *wire.Codec, chainID sdk.Int, msgs []sdk.Msg, pKeys []*ecdsa.PrivateKey, + accNums []int64, seqs []int64, fee auth.StdFee, +) []Transaction { + + txs := make([]Transaction, len(pKeys)) + 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, + sdk.NewInt(100), payload, + ) + + emintTx.Sign(TestChainID, priv) + + txs[i] = emintTx + } + + return txs +} diff --git a/types/tx.go b/types/tx.go index a7043d63e..873a8572e 100644 --- a/types/tx.go +++ b/types/tx.go @@ -68,6 +68,13 @@ type ( EthSignature struct { v, r, s *big.Int } + + // sigCache is used to cache the derived sender and contains + // the signer used to derive it. + sigCache struct { + signer ethtypes.Signer + from ethcmn.Address + } ) // NewEthSignature returns a new instantiated Ethereum signature. @@ -164,6 +171,56 @@ func (tx *Transaction) Sign(chainID sdk.Int, priv *ecdsa.PrivateKey) { tx.Data.Signature.s = s } +func (tx Transaction) VerifySig(chainID *big.Int) (ethcmn.Address, error) { + 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)) { + return sigCache.from, nil + } + } + + var signBytes ethcmn.Hash + if tx.Data.Signature.v.BitLen() < 8 { + v := tx.Data.Signature.v.Uint64() + if v == 27 || v == 28 { + // Unprotected tx has no cross-chain replay protection + signBytes = rlpHash([]interface{}{ + tx.Data.AccountNonce, + tx.Data.Price, + tx.Data.GasLimit, + tx.Data.Recipient, + tx.Data.Amount, + tx.Data.Payload, + }) + } + } else { + signBytes = rlpHash([]interface{}{ + tx.Data.AccountNonce, + tx.Data.Price, + tx.Data.GasLimit, + tx.Data.Recipient, + tx.Data.Amount, + tx.Data.Payload, + chainID, uint(0), uint(0), + }) + } + + sig := recoverEthSig(tx.Data.Signature, chainID) + + pub, err := ethcrypto.Ecrecover(signBytes[:], sig) + if err != nil { + return ethcmn.Address{}, err + } + + + var addr ethcmn.Address + copy(addr[:], ethcrypto.Keccak256(pub[1:])[12:]) + return addr, nil +} + // Type implements the sdk.Msg interface. It returns the type of the // Transaction. func (tx Transaction) Type() string { @@ -200,27 +257,6 @@ func (tx Transaction) GetMsgs() []sdk.Msg { return []sdk.Msg{tx} } -// ConvertTx attempts to converts a Transaction to a new Ethereum transaction -// with the signature set. The signature if first recovered and then a new -// Transaction is created with that signature. If setting the signature fails, -// a panic will be triggered. -func (tx Transaction) ConvertTx(chainID *big.Int) ethtypes.Transaction { - gethTx := ethtypes.NewTransaction( - tx.Data.AccountNonce, *tx.Data.Recipient, tx.Data.Amount.BigInt(), - tx.Data.GasLimit, tx.Data.Price.BigInt(), tx.Data.Payload, - ) - - sig := recoverEthSig(tx.Data.Signature, chainID) - signer := ethtypes.NewEIP155Signer(chainID) - - gethTx, err := gethTx.WithSignature(signer, sig) - if err != nil { - panic(errors.Wrap(err, "failed to convert transaction with a given signature")) - } - - return *gethTx -} - // HasEmbeddedTx returns a boolean reflecting if the transaction contains an // SDK transaction or not based on the recipient address. func (tx Transaction) HasEmbeddedTx(addr ethcmn.Address) bool { diff --git a/types/tx_test.go b/types/tx_test.go index b186f7b9c..7d3ac20ec 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -3,164 +3,18 @@ package types import ( "crypto/ecdsa" "fmt" - "math/big" "testing" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/wire" - "github.com/cosmos/cosmos-sdk/x/auth" ethcmn "github.com/ethereum/go-ethereum/common" - ethtypes "github.com/ethereum/go-ethereum/core/types" - ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" ) -var ( - testChainID = sdk.NewInt(3) - - testPrivKey1, _ = ethcrypto.GenerateKey() - testPrivKey2, _ = ethcrypto.GenerateKey() - - testAddr1 = PrivKeyToEthAddress(testPrivKey1) - testAddr2 = PrivKeyToEthAddress(testPrivKey2) - - testSDKAddress = GenerateEthAddress() -) - -func newTestCodec() *wire.Codec { - codec := wire.NewCodec() - - RegisterWire(codec) - codec.RegisterConcrete(&sdk.TestMsg{}, "test/TestMsg", nil) - - return codec -} - -func newStdFee() auth.StdFee { - return auth.NewStdFee(5000, sdk.NewCoin("photon", 150)) -} - -func newTestEmbeddedTx( - chainID sdk.Int, msgs []sdk.Msg, pKeys []*ecdsa.PrivateKey, - accNums []int64, seqs []int64, fee auth.StdFee, -) sdk.Tx { - - sigs := make([][]byte, len(pKeys)) - - for i, priv := range pKeys { - signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, newStdFee()} - - signBytes, err := signEtx.Bytes() - if err != nil { - panic(err) - } - - sig, err := ethcrypto.Sign(signBytes, priv) - if err != nil { - panic(err) - } - - sigs[i] = sig - } - - return EmbeddedTx{msgs, fee, sigs} -} - -func newTestGethTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Address) []ethtypes.Transaction { - txs := make([]ethtypes.Transaction, len(pKeys)) - - for i, priv := range pKeys { - ethTx := ethtypes.NewTransaction( - uint64(i), addrs[i], big.NewInt(10), 100, big.NewInt(100), nil, - ) - - signer := ethtypes.NewEIP155Signer(chainID.BigInt()) - ethTx, _ = ethtypes.SignTx(ethTx, signer, priv) - - txs[i] = *ethTx - } - - return txs -} - -func newTestEthTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Address) []Transaction { - txs := make([]Transaction, len(pKeys)) - - for i, priv := range pKeys { - emintTx := NewTransaction( - uint64(i), addrs[i], sdk.NewInt(10), 100, sdk.NewInt(100), nil, - ) - - emintTx.Sign(chainID, priv) - - txs[i] = emintTx - } - - return txs -} - -func newTestSDKTxs( - codec *wire.Codec, chainID sdk.Int, msgs []sdk.Msg, pKeys []*ecdsa.PrivateKey, - accNums []int64, seqs []int64, fee auth.StdFee, -) []Transaction { - - txs := make([]Transaction, len(pKeys)) - 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, - sdk.NewInt(100), payload, - ) - - emintTx.Sign(testChainID, priv) - - txs[i] = emintTx - } - - return txs -} - -func TestConvertTx(t *testing.T) { - gethTxs := newTestGethTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1, testPrivKey2}, - []ethcmn.Address{testAddr1, testAddr2}, - ) - ethTxs := newTestEthTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1, testPrivKey2}, - []ethcmn.Address{testAddr1, testAddr2}, - ) - - testCases := []struct { - ethTx ethtypes.Transaction - emintTx Transaction - expectedEq bool - }{ - {gethTxs[0], ethTxs[0], true}, - {gethTxs[0], ethTxs[1], false}, - {gethTxs[1], ethTxs[0], false}, - } - - for i, tc := range testCases { - convertedTx := tc.emintTx.ConvertTx(testChainID.BigInt()) - - if tc.expectedEq { - require.Equal(t, tc.ethTx, convertedTx, fmt.Sprintf("unexpected result: test case #%d", i)) - } else { - require.NotEqual(t, tc.ethTx, convertedTx, fmt.Sprintf("unexpected result: test case #%d", i)) - } - } -} - func TestValidation(t *testing.T) { - ethTxs := newTestEthTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1}, - []ethcmn.Address{testAddr1}, + ethTxs := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, ) testCases := []struct { @@ -194,34 +48,34 @@ func TestValidation(t *testing.T) { } func TestHasEmbeddedTx(t *testing.T) { - testCodec := newTestCodec() - msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + testCodec := NewTestCodec() + msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} - sdkTxs := newTestSDKTxs( - testCodec, testChainID, msgs, []*ecdsa.PrivateKey{testPrivKey1}, + sdkTxs := NewTestSDKTxs( + testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, []int64{0}, []int64{0}, newStdFee(), ) - require.True(t, sdkTxs[0].HasEmbeddedTx(testSDKAddress)) + require.True(t, sdkTxs[0].HasEmbeddedTx(TestSDKAddress)) - ethTxs := newTestEthTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1}, - []ethcmn.Address{testAddr1}, + ethTxs := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, ) - require.False(t, ethTxs[0].HasEmbeddedTx(testSDKAddress)) + require.False(t, ethTxs[0].HasEmbeddedTx(TestSDKAddress)) } func TestGetEmbeddedTx(t *testing.T) { - testCodec := newTestCodec() - msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + testCodec := NewTestCodec() + msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} - ethTxs := newTestEthTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1}, - []ethcmn.Address{testAddr1}, + ethTxs := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, ) - sdkTxs := newTestSDKTxs( - testCodec, testChainID, msgs, []*ecdsa.PrivateKey{testPrivKey1}, + sdkTxs := NewTestSDKTxs( + testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, []int64{0}, []int64{0}, newStdFee(), ) @@ -235,19 +89,19 @@ func TestGetEmbeddedTx(t *testing.T) { } func TestTransactionGetMsgs(t *testing.T) { - ethTxs := newTestEthTxs( - testChainID, - []*ecdsa.PrivateKey{testPrivKey1}, - []ethcmn.Address{testAddr1}, + ethTxs := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, ) msgs := ethTxs[0].GetMsgs() require.Len(t, msgs, 1) require.Equal(t, ethTxs[0], msgs[0]) - expectedMsgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + expectedMsgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( - testChainID, expectedMsgs, []*ecdsa.PrivateKey{testPrivKey1}, + TestChainID, expectedMsgs, []*ecdsa.PrivateKey{TestPrivKey1}, []int64{0}, []int64{0}, newStdFee(), ) @@ -257,26 +111,26 @@ func TestTransactionGetMsgs(t *testing.T) { } func TestGetRequiredSigners(t *testing.T) { - msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( - testChainID, msgs, []*ecdsa.PrivateKey{testPrivKey1}, + TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, []int64{0}, []int64{0}, newStdFee(), ) signers := etx.(EmbeddedTx).GetRequiredSigners() - require.Equal(t, []sdk.AccAddress{sdk.AccAddress(testAddr1.Bytes())}, signers) + require.Equal(t, []sdk.AccAddress{sdk.AccAddress(TestAddr1.Bytes())}, signers) } func TestTxDecoder(t *testing.T) { - testCodec := newTestCodec() - txDecoder := TxDecoder(testCodec, testSDKAddress) - msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + testCodec := NewTestCodec() + txDecoder := TxDecoder(testCodec, TestSDKAddress) + msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} // create a non-SDK Ethereum transaction emintTx := NewTransaction( - uint64(0), testAddr1, sdk.NewInt(10), 100, sdk.NewInt(100), nil, + uint64(0), TestAddr1, sdk.NewInt(10), 100, sdk.NewInt(100), nil, ) - emintTx.Sign(testChainID, testPrivKey1) + emintTx.Sign(TestChainID, TestPrivKey1) // require the transaction to properly decode into a Transaction txBytes := testCodec.MustMarshalBinary(emintTx) @@ -286,7 +140,7 @@ func TestTxDecoder(t *testing.T) { // create embedded transaction and encode etx := newTestEmbeddedTx( - testChainID, msgs, []*ecdsa.PrivateKey{testPrivKey1}, + TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, []int64{0}, []int64{0}, newStdFee(), ) @@ -296,10 +150,10 @@ func TestTxDecoder(t *testing.T) { testCodec.UnmarshalBinary(payload, &expectedEtx) emintTx = NewTransaction( - uint64(0), testSDKAddress, sdk.NewInt(10), 100, + uint64(0), TestSDKAddress, sdk.NewInt(10), 100, sdk.NewInt(100), payload, ) - emintTx.Sign(testChainID, testPrivKey1) + emintTx.Sign(TestChainID, TestPrivKey1) // require the transaction to properly decode into a Transaction txBytes = testCodec.MustMarshalBinary(emintTx) @@ -314,9 +168,9 @@ func TestTxDecoder(t *testing.T) { // create a non-SDK Ethereum transaction with an SDK address and garbage payload emintTx = NewTransaction( - uint64(0), testSDKAddress, sdk.NewInt(10), 100, sdk.NewInt(100), []byte("garbage"), + uint64(0), TestSDKAddress, sdk.NewInt(10), 100, sdk.NewInt(100), []byte("garbage"), ) - emintTx.Sign(testChainID, testPrivKey1) + emintTx.Sign(TestChainID, TestPrivKey1) // require the transaction to fail decoding as the payload is invalid txBytes = testCodec.MustMarshalBinary(emintTx) diff --git a/types/utils_test.go b/types/utils_test.go index cb12c8414..341b66db1 100644 --- a/types/utils_test.go +++ b/types/utils_test.go @@ -10,27 +10,27 @@ import ( ) func TestValidateSigner(t *testing.T) { - msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(testAddr1.Bytes()))} + msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} // create message signing structure - signEtx := EmbeddedTxSign{testChainID.String(), 0, 0, msgs, newStdFee()} + signEtx := EmbeddedTxSign{TestChainID.String(), 0, 0, msgs, newStdFee()} // create signing bytes and sign signBytes, err := signEtx.Bytes() require.NoError(t, err) // require signing not to fail - sig, err := ethcrypto.Sign(signBytes, testPrivKey1) + sig, err := ethcrypto.Sign(signBytes, TestPrivKey1) require.NoError(t, err) // require signature to be valid - err = ValidateSigner(signBytes, sig, testAddr1) + err = ValidateSigner(signBytes, sig, TestAddr1) require.NoError(t, err) - sig, err = ethcrypto.Sign(signBytes, testPrivKey2) + sig, err = ethcrypto.Sign(signBytes, TestPrivKey2) require.NoError(t, err) // require signature to be invalid - err = ValidateSigner(signBytes, sig, testAddr1) + err = ValidateSigner(signBytes, sig, TestAddr1) require.Error(t, err) } From 91f120c55dcdbfd15f99f0e9c4a017b5a131089e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Aug 2018 14:04:19 -0700 Subject: [PATCH 2/5] signature verification not working --- handlers/ante.go | 28 +++++++++++++-- handlers/ante_test.go | 82 +++++++++++++++++++++++++++++++++++++++---- types/test_common.go | 10 ++++-- types/tx.go | 42 ++++++++++------------ types/tx_test.go | 23 +++++++++--- types/utils_test.go | 2 +- types/wire.go | 3 ++ 7 files changed, 148 insertions(+), 42 deletions(-) diff --git a/handlers/ante.go b/handlers/ante.go index a62be2142..7b3b8ed5e 100644 --- a/handlers/ante.go +++ b/handlers/ante.go @@ -32,9 +32,9 @@ type internalAnteHandler func( func AnteHandler(am auth.AccountMapper) sdk.AnteHandler { return func(sdkCtx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { var ( - gasLimit int64 handler internalAnteHandler - ) + gasLimit int64 + ) switch tx := tx.(type) { case types.Transaction: @@ -47,8 +47,15 @@ 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 @@ -92,12 +99,27 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont // validate signature sdkCtx.GasMeter().ConsumeGas(verifySigCost, "ante verify") - _, err := ethTx.VerifySig(chainID) + addr, err := ethTx.VerifySig(chainID) + + fmt.Println(addr) + fmt.Println(chainID) if err != nil { return sdkCtx, sdk.ErrUnauthorized("signature verification failed").Result(), true } + // validate AccountNonce (called Sequence in AccountMapper) + acc := am.GetAccount(sdkCtx, addr[:]) + seq := acc.GetSequence() + if ethTx.Data.AccountNonce != uint64(seq) { + return sdkCtx, sdk.ErrInvalidSequence(fmt.Sprintf("Wrong AccountNonce: expected %d", seq)).Result(), true + } + err = acc.SetSequence(seq + 1) + if err != nil { + panic(err) + } + am.SetAccount(sdkCtx, acc) + return sdkCtx, sdk.Result{GasWanted: int64(ethTx.Data.GasLimit)}, false } diff --git a/handlers/ante_test.go b/handlers/ante_test.go index 2cfc62850..37bb19db7 100644 --- a/handlers/ante_test.go +++ b/handlers/ante_test.go @@ -1,6 +1,7 @@ package handlers import ( + "fmt" "testing" "math/big" "crypto/ecdsa" @@ -8,7 +9,10 @@ import ( "github.com/cosmos/ethermint/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/libs/log" ethcmn "github.com/ethereum/go-ethereum/common" @@ -29,7 +33,7 @@ func TestBadSig(t *testing.T) { _, res, abort := handler(ctx, tx) require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad tx") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) } @@ -47,15 +51,41 @@ func TestInsufficientGas(t *testing.T) { _, res, abort := handler(ctx, tx) require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, "Result is OK on bad tx") + require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) } -func TestWrongNonce(t *testing.T) { +func TestIncorrectNonce(t *testing.T) { tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] tx.Data.AccountNonce = 12 + ms, key := SetupMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, log.NewNopLogger()) + + 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) + + 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") + require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) + +} + +func TestValidTx(t *testing.T) { + tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + ms, key := SetupMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) @@ -64,7 +94,47 @@ func TestWrongNonce(t *testing.T) { _, res, abort := handler(ctx, tx) - require.True(t, abort, "Antehandler did not abort") - require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, "Result is OK on bad 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)) -} \ No newline at end of file +} + +func TestValidEmbeddedTx(t *testing.T) { + cdc := types.NewTestCodec() + // 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] + + 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.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)) +} + +/* +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 ab2aa1119..5c0664a41 100644 --- a/types/test_common.go +++ b/types/test_common.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/auth" + stake "github.com/cosmos/cosmos-sdk/x/stake/types" ethcrypto "github.com/ethereum/go-ethereum/crypto" ethtypes "github.com/ethereum/go-ethereum/core/types" @@ -31,10 +32,13 @@ func NewTestCodec() *wire.Codec { RegisterWire(codec) codec.RegisterConcrete(&sdk.TestMsg{}, "test/TestMsg", nil) + // Register any desired SDK msgs to be embedded + stake.RegisterWire(codec) + return codec } -func newStdFee() auth.StdFee { +func NewStdFee() auth.StdFee { return auth.NewStdFee(5000, sdk.NewCoin("photon", 150)) } @@ -46,7 +50,7 @@ func newTestEmbeddedTx( sigs := make([][]byte, len(pKeys)) for i, priv := range pKeys { - signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, newStdFee()} + signEtx := EmbeddedTxSign{chainID.String(), accNums[i], seqs[i], msgs, fee} signBytes, err := signEtx.Bytes() if err != nil { @@ -86,7 +90,7 @@ func NewTestEthTxs(chainID sdk.Int, pKeys []*ecdsa.PrivateKey, addrs []ethcmn.Ad for i, priv := range pKeys { emintTx := NewTransaction( - uint64(i), addrs[i], sdk.NewInt(10), 100, sdk.NewInt(100), nil, + uint64(i), addrs[i], sdk.NewInt(10), 1000, sdk.NewInt(100), nil, ) emintTx.Sign(chainID, priv) diff --git a/types/tx.go b/types/tx.go index 873a8572e..d29c2f0cf 100644 --- a/types/tx.go +++ b/types/tx.go @@ -182,31 +182,16 @@ func (tx Transaction) VerifySig(chainID *big.Int) (ethcmn.Address, error) { } } - var signBytes ethcmn.Hash - if tx.Data.Signature.v.BitLen() < 8 { - v := tx.Data.Signature.v.Uint64() - if v == 27 || v == 28 { - // Unprotected tx has no cross-chain replay protection - signBytes = rlpHash([]interface{}{ - tx.Data.AccountNonce, - tx.Data.Price, - tx.Data.GasLimit, - tx.Data.Recipient, - tx.Data.Amount, - tx.Data.Payload, - }) - } - } else { - signBytes = rlpHash([]interface{}{ - tx.Data.AccountNonce, - tx.Data.Price, - tx.Data.GasLimit, - tx.Data.Recipient, - tx.Data.Amount, - tx.Data.Payload, - chainID, uint(0), uint(0), + + signBytes := rlpHash([]interface{}{ + tx.Data.AccountNonce, + tx.Data.Price, + tx.Data.GasLimit, + tx.Data.Recipient, + tx.Data.Amount, + tx.Data.Payload, + chainID, uint(0), uint(0), }) - } sig := recoverEthSig(tx.Data.Signature, chainID) @@ -279,6 +264,15 @@ func (tx Transaction) GetEmbeddedTx(codec *wire.Codec) (EmbeddedTx, sdk.Error) { return etx, nil } +// Copies Ethereum tx's Protected function +func (tx Transaction) protected() bool { + if tx.Data.Signature.v.BitLen() <= 8 { + v := tx.Data.Signature.v.Uint64() + return v != 27 && v != 28 + } + return true +} + // ---------------------------------------------------------------------------- // embedded SDK transaction // ---------------------------------------------------------------------------- diff --git a/types/tx_test.go b/types/tx_test.go index 7d3ac20ec..b7a3dcb50 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -53,7 +53,7 @@ func TestHasEmbeddedTx(t *testing.T) { sdkTxs := NewTestSDKTxs( testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) require.True(t, sdkTxs[0].HasEmbeddedTx(TestSDKAddress)) @@ -76,7 +76,7 @@ func TestGetEmbeddedTx(t *testing.T) { ) sdkTxs := NewTestSDKTxs( testCodec, TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) etx, err := sdkTxs[0].GetEmbeddedTx(testCodec) @@ -102,7 +102,7 @@ func TestTransactionGetMsgs(t *testing.T) { expectedMsgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( TestChainID, expectedMsgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) msgs = etx.GetMsgs() @@ -114,13 +114,26 @@ func TestGetRequiredSigners(t *testing.T) { msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} etx := newTestEmbeddedTx( TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) signers := etx.(EmbeddedTx).GetRequiredSigners() require.Equal(t, []sdk.AccAddress{sdk.AccAddress(TestAddr1.Bytes())}, signers) } +func TestVerifySig(t *testing.T) { + ethTx := NewTestEthTxs( + TestChainID, + []*ecdsa.PrivateKey{TestPrivKey1}, + []ethcmn.Address{TestAddr1}, + )[0] + + addr, err := ethTx.VerifySig(TestChainID.BigInt()) + + require.Nil(t, err, "Sig verification failed") + require.Equal(t, TestAddr1, addr, "Address is not the same") +} + func TestTxDecoder(t *testing.T) { testCodec := NewTestCodec() txDecoder := TxDecoder(testCodec, TestSDKAddress) @@ -141,7 +154,7 @@ func TestTxDecoder(t *testing.T) { // create embedded transaction and encode etx := newTestEmbeddedTx( TestChainID, msgs, []*ecdsa.PrivateKey{TestPrivKey1}, - []int64{0}, []int64{0}, newStdFee(), + []int64{0}, []int64{0}, NewStdFee(), ) payload := testCodec.MustMarshalBinary(etx) diff --git a/types/utils_test.go b/types/utils_test.go index 341b66db1..430c5b034 100644 --- a/types/utils_test.go +++ b/types/utils_test.go @@ -13,7 +13,7 @@ func TestValidateSigner(t *testing.T) { msgs := []sdk.Msg{sdk.NewTestMsg(sdk.AccAddress(TestAddr1.Bytes()))} // create message signing structure - signEtx := EmbeddedTxSign{TestChainID.String(), 0, 0, msgs, newStdFee()} + signEtx := EmbeddedTxSign{TestChainID.String(), 0, 0, msgs, NewStdFee()} // create signing bytes and sign signBytes, err := signEtx.Bytes() diff --git a/types/wire.go b/types/wire.go index 5414fcdcf..818b17422 100644 --- a/types/wire.go +++ b/types/wire.go @@ -3,6 +3,7 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" + "github.com/cosmos/cosmos-sdk/x/auth" ) var typesCodec = wire.NewCodec() @@ -15,6 +16,8 @@ func init() { // codec. func RegisterWire(codec *wire.Codec) { sdk.RegisterWire(codec) + wire.RegisterCrypto(codec) + auth.RegisterWire(codec) codec.RegisterConcrete(&EthSignature{}, "types/EthSignature", nil) codec.RegisterConcrete(TxData{}, "types/TxData", nil) codec.RegisterConcrete(Transaction{}, "types/Transaction", nil) From f73ab8a0dae80cc148a15881f7fa91508072f1fb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Aug 2018 15:37:10 -0700 Subject: [PATCH 3/5] Fixed sig issue, wrote embedded tests --- handlers/ante.go | 12 +-- handlers/ante_test.go | 196 ++++++++++++++++++++++++++++++++++-------- types/test_common.go | 6 +- types/tx.go | 14 ++- types/tx_test.go | 6 +- 5 files changed, 178 insertions(+), 56 deletions(-) diff --git a/handlers/ante.go b/handlers/ante.go index 7b3b8ed5e..3a8be0d63 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 37bb19db7..1a5d3a425 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 5c0664a41..d344ede45 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 d29c2f0cf..9fcfe1fe5 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 b7a3dcb50..f4a553ce6 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(), ) From 9164db3e0a70cc8142c3ad5401fc359705288e95 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Aug 2018 15:43:01 -0700 Subject: [PATCH 4/5] fix dep --- Gopkg.lock | 9 +++++---- Gopkg.toml | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f6b028f6f..6d78de474 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -26,7 +26,8 @@ "types", "version", "wire", - "x/auth" + "x/auth", + "x/stake/types" ] revision = "23e3d5ac12145c02fcb4b4767d7dfccad782aee5" version = "v0.23.1" @@ -193,8 +194,8 @@ "assert", "require" ] - revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" - version = "v1.2.2" + revision = "12b6f73e6084dad08a7c6e575284b177ecafbc71" + version = "v1.2.1" [[projects]] branch = "master" @@ -358,6 +359,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "560c44b5e8f495ae409bc13b0e32cf36e3281f0d02b68b15e987faa2cda1e91c" + inputs-digest = "353f6762943df5f67c2fba13bc530c800977ed507155bb9edc3b44947f0abc6e" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 4a6a8e232..1010c541b 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -24,7 +24,7 @@ [[constraint]] name = "github.com/stretchr/testify" - version = "=1.2.2" + version = "=1.2.1" [[constraint]] name = "github.com/pkg/errors" From e83a2b97f91edeced4a1a04030bdabba47458306 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 31 Aug 2018 16:44:34 -0400 Subject: [PATCH 5/5] Update ante handler unit tests --- handlers/ante.go | 22 +-- handlers/ante_test.go | 326 +++++++++++++++++++--------------------- handlers/test_common.go | 4 +- types/test_common.go | 21 +-- types/tx_test.go | 10 +- 5 files changed, 177 insertions(+), 206 deletions(-) diff --git a/handlers/ante.go b/handlers/ante.go index 72f97460f..df9e067c9 100644 --- a/handlers/ante.go +++ b/handlers/ante.go @@ -21,14 +21,14 @@ const ( // must implementing. Internal ante handlers will be dependant upon the // transaction type. type internalAnteHandler func( - sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper, + sdkCtx sdk.Context, tx sdk.Tx, accMapper auth.AccountMapper, ) (newCtx sdk.Context, res sdk.Result, abort bool) // AnteHandler is responsible for attempting to route an Ethereum or SDK // transaction to an internal ante handler for performing transaction-level // processing (e.g. fee payment, signature verification) before being passed // onto it's respective handler. -func AnteHandler(am auth.AccountMapper, _ auth.FeeCollectionKeeper) sdk.AnteHandler { +func AnteHandler(accMapper auth.AccountMapper, _ auth.FeeCollectionKeeper) sdk.AnteHandler { return func(sdkCtx sdk.Context, tx sdk.Tx) (newCtx sdk.Context, res sdk.Result, abort bool) { var ( handler internalAnteHandler @@ -67,7 +67,7 @@ func AnteHandler(am auth.AccountMapper, _ auth.FeeCollectionKeeper) sdk.AnteHand } }() - return handler(newCtx, tx, am) + return handler(newCtx, tx, accMapper) } } @@ -76,7 +76,7 @@ func AnteHandler(am auth.AccountMapper, _ auth.FeeCollectionKeeper) sdk.AnteHand // // TODO: Do we need to do any further validation or account manipulation // (e.g. increment nonce)? -func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Context, sdk.Result, bool) { +func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, accMapper auth.AccountMapper) (sdk.Context, sdk.Result, bool) { ethTx, ok := tx.(types.Transaction) if !ok { return sdkCtx, sdk.ErrInternal(fmt.Sprintf("invalid transaction: %T", tx)).Result(), true @@ -96,7 +96,7 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont return sdkCtx, sdk.ErrUnauthorized("signature verification failed").Result(), true } - acc := am.GetAccount(sdkCtx, addr.Bytes()) + acc := accMapper.GetAccount(sdkCtx, addr.Bytes()) // validate the account nonce (referred to as sequence in the AccountMapper) seq := acc.GetSequence() @@ -109,13 +109,13 @@ func handleEthTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Cont return sdkCtx, sdk.ErrInternal(err.Error()).Result(), true } - am.SetAccount(sdkCtx, acc) + accMapper.SetAccount(sdkCtx, acc) return sdkCtx, sdk.Result{GasWanted: int64(ethTx.Data().GasLimit)}, false } // handleEmbeddedTx implements an ante handler for an SDK transaction. It // validates the signature and if valid returns an OK result. -func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk.Context, sdk.Result, bool) { +func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, accMapper auth.AccountMapper) (sdk.Context, sdk.Result, bool) { stdTx, ok := tx.(auth.StdTx) if !ok { return sdkCtx, sdk.ErrInternal(fmt.Sprintf("invalid transaction: %T", tx)).Result(), true @@ -132,7 +132,7 @@ func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk for i, sig := range stdTx.Signatures { signer := ethcmn.BytesToAddress(signerAddrs[i].Bytes()) - acc, err := validateSignature(sdkCtx, stdTx, signer, sig, am) + acc, err := validateSignature(sdkCtx, stdTx, signer, sig, accMapper) // err.Code() != sdk.CodeOK if err != nil { return sdkCtx, err.Result(), true @@ -140,7 +140,7 @@ func handleEmbeddedTx(sdkCtx sdk.Context, tx sdk.Tx, am auth.AccountMapper) (sdk // TODO: Fees! - am.SetAccount(sdkCtx, acc) + accMapper.SetAccount(sdkCtx, acc) signerAccs[i] = acc } @@ -167,12 +167,12 @@ func validateStdTxBasic(stdTx auth.StdTx) (err sdk.Error) { func validateSignature( sdkCtx sdk.Context, stdTx auth.StdTx, signer ethcmn.Address, - sig auth.StdSignature, am auth.AccountMapper, + sig auth.StdSignature, accMapper auth.AccountMapper, ) (acc auth.Account, sdkErr sdk.Error) { chainID := sdkCtx.ChainID() - acc = am.GetAccount(sdkCtx, signer.Bytes()) + acc = accMapper.GetAccount(sdkCtx, signer.Bytes()) if acc == nil { return nil, sdk.ErrUnknownAddress(fmt.Sprintf("no account with address %s found", signer)) } diff --git a/handlers/ante_test.go b/handlers/ante_test.go index 1a5d3a425..cff445153 100644 --- a/handlers/ante_test.go +++ b/handlers/ante_test.go @@ -1,266 +1,256 @@ package handlers import ( - "fmt" - "testing" - "math/big" "crypto/ecdsa" + "fmt" + "math/big" + "testing" - "github.com/cosmos/ethermint/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" - stake "github.com/cosmos/cosmos-sdk/x/stake/types" - + "github.com/cosmos/cosmos-sdk/x/stake" + "github.com/cosmos/ethermint/types" + ethcrypto "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" - - ethcmn "github.com/ethereum/go-ethereum/common" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -// Test EthTx Antehandler -// ----------------------------------------------------------------------------------------------------------------------------------- +func TestEthTxBadSig(t *testing.T) { + tx := types.NewTransaction(uint64(0), types.TestAddr1, big.NewInt(10), 1000, big.NewInt(100), []byte{}) -func TestBadSig(t *testing.T) { - tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] + // create bad signature + tx.Sign(big.NewInt(100), types.TestPrivKey2) - tx.Data.Signature = types.NewEthSignature(new(big.Int), new(big.Int), new(big.Int)) - - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - handler := AnteHandler(accountMapper) - - _, res, abort := handler(ctx, tx) - - 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)) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) + _, res, abort := handler(ctx, *tx) + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestInsufficientGas(t *testing.T) { - tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] - - tx.Data.GasLimit = 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) - - 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) { - // Create transaction with wrong nonce 12 - tx := types.NewTransaction(12, types.TestAddr2, sdk.NewInt(50), 1000, sdk.NewInt(1000), []byte("test_bytes")) +func TestEthTxInsufficientGas(t *testing.T) { + tx := types.NewTransaction(uint64(0), types.TestAddr1, big.NewInt(0), 0, big.NewInt(0), []byte{}) tx.Sign(types.TestChainID, types.TestPrivKey1) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() + ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) + _, res, abort := handler(ctx, *tx) + + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x1000c), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) +} + +func TestEthTxIncorrectNonce(t *testing.T) { + // create transaction with wrong nonce 12 + tx := types.NewTransaction(12, types.TestAddr2, big.NewInt(50), 1000, big.NewInt(1000), []byte("test_bytes")) + tx.Sign(types.TestChainID, types.TestPrivKey1) + + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, log.NewNopLogger()) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + // set account in accountMapper + acc := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) + accMapper.SetAccount(ctx, acc) - // 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") - require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("Result has wrong code on bad tx: %s", res.Log)) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) + _, res, abort := handler(ctx, *tx) + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestValidTx(t *testing.T) { - tx := types.NewTestEthTxs(types.TestChainID, []*ecdsa.PrivateKey{types.TestPrivKey1}, []ethcmn.Address{types.TestAddr1})[0] +func TestEthTxValidTx(t *testing.T) { + tx := types.NewTransaction(0, types.TestAddr1, big.NewInt(50), 1000, big.NewInt(1000), []byte{}) + tx.Sign(types.TestChainID, types.TestPrivKey1) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + // set account in accountMapper + acc := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) + accMapper.SetAccount(ctx, acc) - // Set account in accountMapper - acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:]) - accountMapper.SetAccount(ctx, acc) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) + _, res, abort := handler(ctx, *tx) - handler := AnteHandler(accountMapper) + require.False(t, abort, "expected ante handler to not abort") + require.True(t, res.IsOK(), fmt.Sprintf("result not OK on valid Tx: %s", res.Log)) - _, res, abort := handler(ctx, tx) - - 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") + // ensure account state updated correctly + seq, _ := accMapper.GetSequence(ctx, types.TestAddr1[:]) + require.Equal(t, int64(1), seq, "account nonce did not increment correctly") } -// Test EmbeddedTx Antehandler -// ----------------------------------------------------------------------------------------------------------------------------------- +func TestEmbeddedTxBadSig(t *testing.T) { + testCodec := types.NewTestCodec() + testFee := types.NewTestStdFee() -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)})} + msgs := []sdk.Msg{sdk.NewTestMsg()} + tx := types.NewTestStdTx( + types.TestChainID, msgs, []int64{0}, []int64{0}, []*ecdsa.PrivateKey{types.TestPrivKey1}, testFee, + ) - // Create transaction with incorrect signer - tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey2}, - []int64{0}, []int64{0}, types.NewStdFee()) + // create bad signature + signBytes := types.GetStdTxSignBytes(big.NewInt(100).String(), 1, 1, testFee, msgs, "") + sig, _ := ethcrypto.Sign(signBytes, types.TestPrivKey1) + (tx.(auth.StdTx)).Signatures[0].Signature = sig - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(testCodec, key, auth.ProtoBaseAccount) - 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) + // set account in accountMapper + acc := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) + accMapper.SetAccount(ctx, acc) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) _, 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") + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestInvalidMultiMsgEmbeddedTx(t *testing.T) { - // Create msg to be embedded +func TestEmbeddedTxInvalidMultiMsg(t *testing.T) { + testCodec := types.NewTestCodec() + testCodec.RegisterConcrete(stake.MsgDelegate{}, "test/MsgDelegate", nil) + 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)}), + stake.NewMsgDelegate(types.TestAddr1.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("steak", sdk.NewInt(50))), + stake.NewMsgDelegate(types.TestAddr2.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("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()) + // create transaction with only one signer + tx := types.NewTestStdTx( + types.TestChainID, msgs, []int64{0}, []int64{0}, []*ecdsa.PrivateKey{types.TestPrivKey1}, types.NewTestStdFee(), + ) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(testCodec, key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) + // set accounts in accountMapper + acc1 := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) + accMapper.SetAccount(ctx, acc1) - // 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) + acc2 := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) + accMapper.SetAccount(ctx, acc2) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) _, 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") + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10004), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestInvalidAccountNumberEmbeddedTx(t *testing.T) { - // Create msg to be embedded +func TestEmbeddedTxInvalidAccountNumber(t *testing.T) { + testCodec := types.NewTestCodec() + testCodec.RegisterConcrete(stake.MsgDelegate{}, "test/MsgDelegate", nil) + msgs := []sdk.Msg{ - stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}), + stake.NewMsgDelegate(types.TestAddr1.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("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()) + // create a transaction with an invalid account number + tx := types.NewTestStdTx( + types.TestChainID, msgs, []int64{3}, []int64{0}, []*ecdsa.PrivateKey{types.TestPrivKey1}, types.NewTestStdFee(), + ) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(testCodec, key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - - // Set account in accountMapper - acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:]) + // set account in accountMapper + acc := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) acc.SetAccountNumber(4) - accountMapper.SetAccount(ctx, acc) - - handler := AnteHandler(accountMapper) + accMapper.SetAccount(ctx, acc) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) _, 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") + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestInvalidSequenceEmbeddedTx(t *testing.T) { - // Create msg to be embedded +func TestEmbeddedTxInvalidSequence(t *testing.T) { + testCodec := types.NewTestCodec() + testCodec.RegisterConcrete(stake.MsgDelegate{}, "test/MsgDelegate", nil) + msgs := []sdk.Msg{ - stake.NewMsgDelegate(types.TestAddr1[:], types.TestAddr2[:], sdk.Coin{"steak", sdk.NewInt(50)}), + stake.NewMsgDelegate(types.TestAddr1.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("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()) + // create transaction with an invalid sequence (nonce) + tx := types.NewTestStdTx( + types.TestChainID, msgs, []int64{4}, []int64{2}, []*ecdsa.PrivateKey{types.TestPrivKey1}, types.NewTestStdFee(), + ) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - - // Set account in accountMapper - acc := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:]) + // set account in accountMapper + acc := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) acc.SetAccountNumber(4) acc.SetSequence(3) - accountMapper.SetAccount(ctx, acc) - - handler := AnteHandler(accountMapper) + accMapper.SetAccount(ctx, acc) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) _, 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") + require.True(t, abort, "expected ante handler to abort") + require.Equal(t, sdk.ABCICodeType(0x10003), res.Code, fmt.Sprintf("invalid code returned on bad tx: %s", res.Log)) } -func TestValidEmbeddedTx(t *testing.T) { - // Create msg to be embedded +func TestEmbeddedTxValid(t *testing.T) { + testCodec := types.NewTestCodec() + testCodec.RegisterConcrete(stake.MsgDelegate{}, "test/MsgDelegate", nil) + 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)}), + stake.NewMsgDelegate(types.TestAddr1.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("steak", sdk.NewInt(50))), + stake.NewMsgDelegate(types.TestAddr2.Bytes(), types.TestAddr2.Bytes(), sdk.NewCoin("steak", sdk.NewInt(50))), } - tx := types.NewTestEmbeddedTx(types.TestChainID, msgs, []*ecdsa.PrivateKey{types.TestPrivKey1, types.TestPrivKey2}, - []int64{4, 5}, []int64{3, 1}, types.NewStdFee()) + // create a valid transaction + tx := types.NewTestStdTx( + types.TestChainID, msgs, []int64{4, 5}, []int64{3, 1}, + []*ecdsa.PrivateKey{types.TestPrivKey1, types.TestPrivKey2}, types.NewTestStdFee(), + ) - ms, key := SetupMultiStore() + ms, key := createTestMultiStore() ctx := sdk.NewContext(ms, abci.Header{ChainID: types.TestChainID.String()}, false, nil) + accMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - accountMapper := auth.NewAccountMapper(types.NewTestCodec(), key, auth.ProtoBaseAccount) - - // Set account in accountMapper - acc1 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr1[:]) + // set accounts in the accountMapper + acc1 := accMapper.NewAccountWithAddress(ctx, types.TestAddr1.Bytes()) acc1.SetAccountNumber(4) acc1.SetSequence(3) - accountMapper.SetAccount(ctx, acc1) - acc2 := accountMapper.NewAccountWithAddress(ctx, types.TestAddr2[:]) + accMapper.SetAccount(ctx, acc1) + + acc2 := accMapper.NewAccountWithAddress(ctx, types.TestAddr2.Bytes()) acc2.SetAccountNumber(5) acc2.SetSequence(1) - accountMapper.SetAccount(ctx, acc2) - - handler := AnteHandler(accountMapper) + accMapper.SetAccount(ctx, acc2) + handler := AnteHandler(accMapper, auth.FeeCollectionKeeper{}) _, res, abort := handler(ctx, tx) - require.False(t, abort, "Antehandler abort on valid embedded tx") - require.True(t, res.IsOK(), fmt.Sprintf("Result not OK on valid Tx: %s", res.Log)) + require.False(t, abort, "expected ante handler to not abort") + 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") + seq1, _ := accMapper.GetSequence(ctx, types.TestAddr1.Bytes()) + seq2, _ := accMapper.GetSequence(ctx, types.TestAddr2.Bytes()) + require.Equal(t, int64(4), seq1, "account nonce did not increment correctly") + require.Equal(t, int64(2), seq2, "account nonce did not increment correctly") } - - - diff --git a/handlers/test_common.go b/handlers/test_common.go index 901f01e9c..05af90f93 100644 --- a/handlers/test_common.go +++ b/handlers/test_common.go @@ -6,11 +6,13 @@ import ( dbm "github.com/tendermint/tendermint/libs/db" ) -func SetupMultiStore() (sdk.MultiStore, *sdk.KVStoreKey) { +func createTestMultiStore() (sdk.MultiStore, *sdk.KVStoreKey) { db := dbm.NewMemDB() capKey := sdk.NewKVStoreKey("capkey") ms := store.NewCommitMultiStore(db) + ms.MountStoreWithDB(capKey, sdk.StoreTypeIAVL, db) ms.LoadLatestVersion() + return ms, capKey } diff --git a/types/test_common.go b/types/test_common.go index 6dc76eb6f..155debc5a 100644 --- a/types/test_common.go +++ b/types/test_common.go @@ -40,7 +40,7 @@ func NewTestStdFee() auth.StdFee { } func NewTestStdTx( - chainID *big.Int, msgs []sdk.Msg, accNums []int64, seqs []int64, pKeys []*ecdsa.PrivateKey, fee auth.StdFee, + chainID *big.Int, msgs []sdk.Msg, accNums, seqs []int64, pKeys []*ecdsa.PrivateKey, fee auth.StdFee, ) sdk.Tx { sigs := make([]auth.StdSignature, len(pKeys)) @@ -100,22 +100,3 @@ func NewTestEthTxs( return txs } - -func NewTestSDKTxs( - codec *wire.Codec, chainID *big.Int, to ethcmn.Address, msgs []sdk.Msg, - accNums []int64, seqs []int64, pKeys []*ecdsa.PrivateKey, fee auth.StdFee, -) []*Transaction { - - txs := make([]*Transaction, len(pKeys)) - stdTx := NewTestStdTx(chainID, msgs, accNums, seqs, pKeys, fee) - payload := codec.MustMarshalBinary(stdTx) - - for i, privKey := range pKeys { - ethTx := NewTransaction(uint64(seqs[i]), to, big.NewInt(10), 1000, big.NewInt(100), payload) - - ethTx.Sign(chainID, privKey) - txs[i] = ethTx - } - - return txs -} diff --git a/types/tx_test.go b/types/tx_test.go index 0df8a8a76..abe74bb7b 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -120,16 +120,14 @@ func TestTxDecoder(t *testing.T) { require.Equal(t, txs[0].data, (decodedTx.(Transaction)).data) // create a SDK (auth.StdTx) transaction and encode - txs = NewTestSDKTxs( - testCodec, TestChainID, TestSDKAddr, msgs, []int64{0}, []int64{0}, - []*ecdsa.PrivateKey{TestPrivKey1}, NewTestStdFee(), - ) + stdTx := NewTestStdTx(TestChainID, msgs, []int64{0}, []int64{0}, []*ecdsa.PrivateKey{TestPrivKey1}, NewTestStdFee()) + payload := testCodec.MustMarshalBinary(stdTx) + tx := NewTransaction(0, TestSDKAddr, big.NewInt(10), 1000, big.NewInt(100), payload) - txBytes, err = rlp.EncodeToBytes(txs[0]) + txBytes, err = rlp.EncodeToBytes(tx) require.NoError(t, err) // require the transaction to properly decode into a Transaction - stdTx := NewTestStdTx(TestChainID, msgs, []int64{0}, []int64{0}, []*ecdsa.PrivateKey{TestPrivKey1}, NewTestStdFee()) decodedTx, err = txDecoder(txBytes) require.NoError(t, err) require.IsType(t, auth.StdTx{}, decodedTx)