diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fa2ca3b2..95449b591 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ BREAKING CHANGES * [stake] remove Tick and add EndBlocker FEATURES +* [x/auth] Added AccountNumbers to BaseAccount and StdTxs to allow for replay protection with account pruning IMPROVEMENTS * bank module uses go-wire codec instead of 'encoding/json' diff --git a/Gopkg.lock b/Gopkg.lock index 612e8e3b3..07a3c6d5b 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -197,8 +197,8 @@ ".", "mem" ] - revision = "63644898a8da0bc22138abf860edaf5277b6102e" - version = "v1.1.0" + revision = "787d034dfe70e44075ccc060d346146ef53270ad" + version = "v1.1.1" [[projects]] name = "github.com/spf13/cast" @@ -236,8 +236,8 @@ "assert", "require" ] - revision = "12b6f73e6084dad08a7c6e575284b177ecafbc71" - version = "v1.2.1" + revision = "f35b8ab0b5a2cef36673838d662e249dd9c94686" + version = "v1.2.2" [[projects]] branch = "master" @@ -366,7 +366,7 @@ "merkle", "merkle/tmhash" ] - revision = "640af0205d98d1f45fb2f912f9c35c8bf816adc9" + revision = "0c98d10b4ffbd87978d79c160e835b3d3df241ec" [[projects]] branch = "master" @@ -396,13 +396,13 @@ "internal/timeseries", "trace" ] - revision = "1e491301e022f8f977054da4c2d852decd59571f" + revision = "db08ff08e8622530d9ed3a0e8ac279f6d4c02196" [[projects]] branch = "master" name = "golang.org/x/sys" packages = ["unix"] - revision = "9527bec2660bd847c050fda93a0f0c6dee0800bb" + revision = "bff228c7b664c5fce602223a05fb708fd8654986" [[projects]] name = "golang.org/x/text" diff --git a/client/context/helpers.go b/client/context/helpers.go index 880c4adb3..0229827fe 100644 --- a/client/context/helpers.go +++ b/client/context/helpers.go @@ -109,12 +109,15 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w if chainID == "" { return nil, errors.Errorf("Chain ID required but not specified") } + accnum := ctx.AccountNumber sequence := ctx.Sequence + signMsg := auth.StdSignMsg{ - ChainID: chainID, - Sequences: []int64{sequence}, - Msg: msg, - Fee: auth.NewStdFee(ctx.Gas, sdk.Coin{}), // TODO run simulate to estimate gas? + ChainID: chainID, + AccountNumbers: []int64{accnum}, + Sequences: []int64{sequence}, + Msg: msg, + Fee: auth.NewStdFee(ctx.Gas, sdk.Coin{}), // TODO run simulate to estimate gas? } keybase, err := keys.GetKeyBase() @@ -130,9 +133,10 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w return nil, err } sigs := []auth.StdSignature{{ - PubKey: pubkey, - Signature: sig, - Sequence: sequence, + PubKey: pubkey, + Signature: sig, + AccountNumber: accnum, + Sequence: sequence, }} // marshal bytes @@ -144,6 +148,10 @@ func (ctx CoreContext) SignAndBuild(name, passphrase string, msg sdk.Msg, cdc *w // sign and build the transaction from the msg func (ctx CoreContext) EnsureSignBuildBroadcast(name string, msg sdk.Msg, cdc *wire.Codec) (res *ctypes.ResultBroadcastTxCommit, err error) { + ctx, err = EnsureAccountNumber(ctx) + if err != nil { + return nil, err + } // default to next sequence number if none provided ctx, err = EnsureSequence(ctx) if err != nil { @@ -163,13 +171,37 @@ func (ctx CoreContext) EnsureSignBuildBroadcast(name string, msg sdk.Msg, cdc *w return ctx.BroadcastTx(txBytes) } +// get the next sequence for the account address +func (ctx CoreContext) GetAccountNumber(address []byte) (int64, error) { + if ctx.Decoder == nil { + return 0, errors.New("AccountDecoder required but not provided") + } + + res, err := ctx.Query(auth.AddressStoreKey(address), ctx.AccountStore) + if err != nil { + return 0, err + } + + if len(res) == 0 { + fmt.Printf("No account found. Returning 0.\n") + return 0, err + } + + account, err := ctx.Decoder(res) + if err != nil { + panic(err) + } + + return account.GetAccountNumber(), nil +} + // get the next sequence for the account address func (ctx CoreContext) NextSequence(address []byte) (int64, error) { if ctx.Decoder == nil { return 0, errors.New("AccountDecoder required but not provided") } - res, err := ctx.Query(address, ctx.AccountStore) + res, err := ctx.Query(auth.AddressStoreKey(address), ctx.AccountStore) if err != nil { return 0, err } diff --git a/client/context/types.go b/client/context/types.go index e9c97ffbc..791ffb23a 100644 --- a/client/context/types.go +++ b/client/context/types.go @@ -14,6 +14,7 @@ type CoreContext struct { TrustNode bool NodeURI string FromAddressName string + AccountNumber int64 Sequence int64 Client rpcclient.Client Decoder auth.AccountDecoder @@ -57,6 +58,12 @@ func (c CoreContext) WithFromAddressName(fromAddressName string) CoreContext { return c } +// WithSequence - return a copy of the context with an account number +func (c CoreContext) WithAccountNumber(accnum int64) CoreContext { + c.AccountNumber = accnum + return c +} + // WithSequence - return a copy of the context with an updated sequence number func (c CoreContext) WithSequence(sequence int64) CoreContext { c.Sequence = sequence diff --git a/client/context/viper.go b/client/context/viper.go index 081c9f5c2..5f262d56f 100644 --- a/client/context/viper.go +++ b/client/context/viper.go @@ -34,6 +34,7 @@ func NewCoreContextFromViper() CoreContext { TrustNode: viper.GetBool(client.FlagTrustNode), FromAddressName: viper.GetString(client.FlagName), NodeURI: nodeURI, + AccountNumber: viper.GetInt64(client.FlagAccountNumber), Sequence: viper.GetInt64(client.FlagSequence), Client: rpc, Decoder: nil, @@ -54,6 +55,25 @@ func defaultChainID() (string, error) { return doc.ChainID, nil } +// EnsureSequence - automatically set sequence number if none provided +func EnsureAccountNumber(ctx CoreContext) (CoreContext, error) { + // Should be viper.IsSet, but this does not work - https://github.com/spf13/viper/pull/331 + if viper.GetInt64(client.FlagAccountNumber) != 0 { + return ctx, nil + } + from, err := ctx.GetFromAddress() + if err != nil { + return ctx, err + } + accnum, err := ctx.GetAccountNumber(from) + if err != nil { + return ctx, err + } + fmt.Printf("Defaulting to account number: %d\n", accnum) + ctx = ctx.WithAccountNumber(accnum) + return ctx, nil +} + // EnsureSequence - automatically set sequence number if none provided func EnsureSequence(ctx CoreContext) (CoreContext, error) { // Should be viper.IsSet, but this does not work - https://github.com/spf13/viper/pull/331 diff --git a/client/flags.go b/client/flags.go index 2d68d3146..4991b9a77 100644 --- a/client/flags.go +++ b/client/flags.go @@ -4,14 +4,15 @@ import "github.com/spf13/cobra" // nolint const ( - FlagChainID = "chain-id" - FlagNode = "node" - FlagHeight = "height" - FlagGas = "gas" - FlagTrustNode = "trust-node" - FlagName = "name" - FlagSequence = "sequence" - FlagFee = "fee" + FlagChainID = "chain-id" + FlagNode = "node" + FlagHeight = "height" + FlagGas = "gas" + FlagTrustNode = "trust-node" + FlagName = "name" + FlagAccountNumber = "account-number" + FlagSequence = "sequence" + FlagFee = "fee" ) // LineBreak can be included in a command list to provide a blank line @@ -34,6 +35,7 @@ func GetCommands(cmds ...*cobra.Command) []*cobra.Command { func PostCommands(cmds ...*cobra.Command) []*cobra.Command { for _, c := range cmds { c.Flags().String(FlagName, "", "Name of private key with which to sign") + c.Flags().Int64(FlagAccountNumber, 0, "AccountNumber number to sign the tx") c.Flags().Int64(FlagSequence, 0, "Sequence number to sign the tx") c.Flags().String(FlagFee, "", "Fee to pay along with transaction") c.Flags().String(FlagChainID, "", "Chain ID of tendermint node") diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index 48e2ad0a2..946f70bbc 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -423,12 +423,14 @@ func doSend(t *testing.T, port, seed, name, password string, addr sdk.Address) ( receiveAddrBech := sdk.MustBech32ifyAcc(receiveAddr) acc := getAccount(t, port, addr) + accnum := acc.GetAccountNumber() sequence := acc.GetSequence() // send jsonStr := []byte(fmt.Sprintf(`{ "name":"%s", - "password":"%s", + "password":"%s", + "account_number":%d, "sequence":%d, "gas": 10000, "amount":[ @@ -437,7 +439,7 @@ func doSend(t *testing.T, port, seed, name, password string, addr sdk.Address) ( "amount": 1 } ] - }`, name, password, sequence, "steak")) + }`, name, password, accnum, sequence, "steak")) res, body := Request(t, port, "POST", "/accounts/"+receiveAddrBech+"/send", jsonStr) require.Equal(t, http.StatusOK, res.StatusCode, body) @@ -457,12 +459,14 @@ func doIBCTransfer(t *testing.T, port, seed, name, password string, addr sdk.Add // get the account to get the sequence acc := getAccount(t, port, addr) + accnum := acc.GetAccountNumber() sequence := acc.GetSequence() // send jsonStr := []byte(fmt.Sprintf(`{ "name":"%s", "password": "%s", + "account_number":%d, "sequence": %d, "gas": 100000, "amount":[ @@ -471,7 +475,7 @@ func doIBCTransfer(t *testing.T, port, seed, name, password string, addr sdk.Add "amount": 1 } ] - }`, name, password, sequence, "steak")) + }`, name, password, accnum, sequence, "steak")) res, body := Request(t, port, "POST", "/ibc/testchain/"+receiveAddrBech+"/send", jsonStr) require.Equal(t, http.StatusOK, res.StatusCode, body) @@ -498,6 +502,7 @@ func getDelegation(t *testing.T, port string, delegatorAddr, validatorAddr sdk.A func doBond(t *testing.T, port, seed, name, password string, delegatorAddr, validatorAddr sdk.Address) (resultTx ctypes.ResultBroadcastTxCommit) { // get the account to get the sequence acc := getAccount(t, port, delegatorAddr) + accnum := acc.GetAccountNumber() sequence := acc.GetSequence() delegatorAddrBech := sdk.MustBech32ifyAcc(delegatorAddr) @@ -507,6 +512,7 @@ func doBond(t *testing.T, port, seed, name, password string, delegatorAddr, vali jsonStr := []byte(fmt.Sprintf(`{ "name": "%s", "password": "%s", + "account_number": %d, "sequence": %d, "gas": 10000, "delegate": [ @@ -517,7 +523,7 @@ func doBond(t *testing.T, port, seed, name, password string, delegatorAddr, vali } ], "unbond": [] - }`, name, password, sequence, delegatorAddrBech, validatorAddrBech, "steak")) + }`, name, password, accnum, sequence, delegatorAddrBech, validatorAddrBech, "steak")) res, body := Request(t, port, "POST", "/stake/delegations", jsonStr) require.Equal(t, http.StatusOK, res.StatusCode, body) @@ -531,6 +537,7 @@ func doBond(t *testing.T, port, seed, name, password string, delegatorAddr, vali func doUnbond(t *testing.T, port, seed, name, password string, delegatorAddr, validatorAddr sdk.Address) (resultTx ctypes.ResultBroadcastTxCommit) { // get the account to get the sequence acc := getAccount(t, port, delegatorAddr) + accnum := acc.GetAccountNumber() sequence := acc.GetSequence() delegatorAddrBech := sdk.MustBech32ifyAcc(delegatorAddr) @@ -540,6 +547,7 @@ func doUnbond(t *testing.T, port, seed, name, password string, delegatorAddr, va jsonStr := []byte(fmt.Sprintf(`{ "name": "%s", "password": "%s", + "account_number": %d, "sequence": %d, "gas": 10000, "delegate": [], @@ -550,7 +558,7 @@ func doUnbond(t *testing.T, port, seed, name, password string, delegatorAddr, va "shares": "30" } ] - }`, name, password, sequence, delegatorAddrBech, validatorAddrBech)) + }`, name, password, accnum, sequence, delegatorAddrBech, validatorAddrBech)) res, body := Request(t, port, "POST", "/stake/delegations", jsonStr) require.Equal(t, http.StatusOK, res.StatusCode, body) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index f06ed2be3..627eee763 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -82,7 +82,8 @@ func NewGaiaApp(logger log.Logger, db dbm.DB) *GaiaApp { app.Router(). AddRoute("bank", bank.NewHandler(app.coinKeeper)). AddRoute("ibc", ibc.NewHandler(app.ibcMapper, app.coinKeeper)). - AddRoute("stake", stake.NewHandler(app.stakeKeeper)) + AddRoute("stake", stake.NewHandler(app.stakeKeeper)). + AddRoute("slashing", slashing.NewHandler(app.slashingKeeper)) // initialize BaseApp app.SetInitChainer(app.initChainer) @@ -144,6 +145,7 @@ func (app *GaiaApp) initChainer(ctx sdk.Context, req abci.RequestInitChain) abci // load the accounts for _, gacc := range genesisState.Accounts { acc := gacc.ToAccount() + acc.AccountNumber = app.accountMapper.GetNextAccountNumber(ctx) app.accountMapper.SetAccount(ctx, acc) } diff --git a/cmd/gaia/app/app_test.go b/cmd/gaia/app/app_test.go index f79703fef..0523c5499 100644 --- a/cmd/gaia/app/app_test.go +++ b/cmd/gaia/app/app_test.go @@ -1,21 +1,11 @@ package app import ( - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/wire" "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/stake" abci "github.com/tendermint/abci/types" - crypto "github.com/tendermint/go-crypto" - dbm "github.com/tendermint/tmlibs/db" - "github.com/tendermint/tmlibs/log" ) func setGenesis(gapp *GaiaApp, accs ...*auth.BaseAccount) error { @@ -41,33 +31,3 @@ func setGenesis(gapp *GaiaApp, accs ...*auth.BaseAccount) error { return nil } - -func TestGenesis(t *testing.T) { - logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app") - db := dbm.NewMemDB() - gapp := NewGaiaApp(logger, db) - - // Construct some genesis bytes to reflect GaiaAccount - pk := crypto.GenPrivKeyEd25519().PubKey() - addr := pk.Address() - coins, err := sdk.ParseCoins("77foocoin,99barcoin") - require.Nil(t, err) - baseAcc := &auth.BaseAccount{ - Address: addr, - Coins: coins, - } - - err = setGenesis(gapp, baseAcc) - require.Nil(t, err) - - // A checkTx context - ctx := gapp.BaseApp.NewContext(true, abci.Header{}) - res1 := gapp.accountMapper.GetAccount(ctx, baseAcc.Address) - assert.Equal(t, baseAcc, res1) - - // reload app and ensure the account is still there - gapp = NewGaiaApp(logger, db) - ctx = gapp.BaseApp.NewContext(true, abci.Header{}) - res1 = gapp.accountMapper.GetAccount(ctx, baseAcc.Address) - assert.Equal(t, baseAcc, res1) -} diff --git a/docs/sdk/overview.rst b/docs/sdk/overview.rst index 8a1350906..0cb7e7304 100644 --- a/docs/sdk/overview.rst +++ b/docs/sdk/overview.rst @@ -232,12 +232,14 @@ a standard form: type StdSignature struct { crypto.PubKey // optional crypto.Signature - Sequence int64 + AccountNumber int64 + Sequence int64 } -It contains the signature itself, as well as the corresponding account's -sequence number. The sequence number is expected to increment every time a -message is signed by a given account. This prevents "replay attacks", where +It contains the signature itself, as well as the corresponding account's account and +sequence numbers. The sequence number is expected to increment every time a +message is signed by a given account. The account number stays the same and is assigned +when the account is first generated. These prevent "replay attacks", where the same message could be executed over and over again. The ``StdSignature`` can also optionally include the public key for verifying the diff --git a/examples/basecoin/app/app.go b/examples/basecoin/app/app.go index fdb0638b6..f654ba05e 100644 --- a/examples/basecoin/app/app.go +++ b/examples/basecoin/app/app.go @@ -146,6 +146,7 @@ func (app *BasecoinApp) initChainer(ctx sdk.Context, req abci.RequestInitChain) panic(err) // TODO https://github.com/cosmos/cosmos-sdk/issues/468 // return sdk.ErrGenesisParse("").TraceCause(err, "") } + acc.AccountNumber = app.accountMapper.GetNextAccountNumber(ctx) app.accountMapper.SetAccount(ctx, acc) } diff --git a/examples/democoin/x/cool/app_test.go b/examples/democoin/x/cool/app_test.go index d41c8ea82..8d3f347b3 100644 --- a/examples/democoin/x/cool/app_test.go +++ b/examples/democoin/x/cool/app_test.go @@ -89,17 +89,17 @@ func TestMsgQuiz(t *testing.T) { assert.Equal(t, acc1, res1) // Set the trend, submit a really cool quiz and check for reward - mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg1, []int64{0}, true, priv1) - mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{1}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg1, []int64{0}, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{0}, []int64{1}, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", 69}}) - mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg2, []int64{2}, false, priv1) // result without reward + mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg2, []int64{0}, []int64{2}, false, priv1) // result without reward mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", 69}}) - mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{3}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{0}, []int64{3}, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", 138}}) - mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg2, []int64{4}, true, priv1) // reset the trend - mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{5}, false, priv1) // the same answer will nolonger do! + mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg2, []int64{0}, []int64{4}, true, priv1) // reset the trend + mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg1, []int64{0}, []int64{5}, false, priv1) // the same answer will nolonger do! mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"icecold", 138}}) - mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg2, []int64{6}, true, priv1) // earlier answer now relavent again + mock.SignCheckDeliver(t, mapp.BaseApp, quizMsg2, []int64{0}, []int64{6}, true, priv1) // earlier answer now relavent again mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"badvibesonly", 69}, {"icecold", 138}}) - mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg3, []int64{7}, false, priv1) // expect to fail to set the trend to something which is not cool + mock.SignCheckDeliver(t, mapp.BaseApp, setTrendMsg3, []int64{0}, []int64{7}, false, priv1) // expect to fail to set the trend to something which is not cool } diff --git a/examples/democoin/x/pow/app_test.go b/examples/democoin/x/pow/app_test.go index 0539df556..aa71fb080 100644 --- a/examples/democoin/x/pow/app_test.go +++ b/examples/democoin/x/pow/app_test.go @@ -71,13 +71,13 @@ func TestMsgMine(t *testing.T) { // Mine and check for reward mineMsg1 := GenerateMsgMine(addr1, 1, 2) - mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg1, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg1, []int64{0}, []int64{0}, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", 1}}) // Mine again and check for reward mineMsg2 := GenerateMsgMine(addr1, 2, 3) - mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg2, []int64{1}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg2, []int64{0}, []int64{1}, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", 2}}) // Mine again - should be invalid - mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg2, []int64{1}, false, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, mineMsg2, []int64{0}, []int64{1}, false, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"pow", 2}}) } diff --git a/x/auth/account.go b/x/auth/account.go index 0ae72a8a6..77966b8e7 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -17,6 +17,9 @@ type Account interface { GetPubKey() crypto.PubKey // can return nil. SetPubKey(crypto.PubKey) error + GetAccountNumber() int64 + SetAccountNumber(int64) error + GetSequence() int64 SetSequence(int64) error @@ -36,10 +39,11 @@ var _ Account = (*BaseAccount)(nil) // Extend this by embedding this in your AppAccount. // See the examples/basecoin/types/account.go for an example. type BaseAccount struct { - Address sdk.Address `json:"address"` - Coins sdk.Coins `json:"coins"` - PubKey crypto.PubKey `json:"public_key"` - Sequence int64 `json:"sequence"` + Address sdk.Address `json:"address"` + Coins sdk.Coins `json:"coins"` + PubKey crypto.PubKey `json:"public_key"` + AccountNumber int64 `json:"account_number"` + Sequence int64 `json:"sequence"` } func NewBaseAccountWithAddress(addr sdk.Address) BaseAccount { @@ -84,6 +88,17 @@ func (acc *BaseAccount) SetCoins(coins sdk.Coins) error { return nil } +// Implements Account +func (acc *BaseAccount) GetAccountNumber() int64 { + return acc.AccountNumber +} + +// Implements Account +func (acc *BaseAccount) SetAccountNumber(accNumber int64) error { + acc.AccountNumber = accNumber + return nil +} + // Implements sdk.Account. func (acc *BaseAccount) GetSequence() int64 { return acc.Sequence diff --git a/x/auth/ante.go b/x/auth/ante.go index 9663bcfe4..c50da0c32 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -14,7 +14,7 @@ const ( ) // NewAnteHandler returns an AnteHandler that checks -// and increments sequence numbers, checks signatures, +// and increments sequence numbers, checks signatures & account numbers, // and deducts fees from the first signer. func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { @@ -46,11 +46,15 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { true } - // Get the sign bytes (requires all sequence numbers and the fee) + // Get the sign bytes (requires all account & sequence numbers and the fee) sequences := make([]int64, len(signerAddrs)) for i := 0; i < len(signerAddrs); i++ { sequences[i] = sigs[i].Sequence } + accNums := make([]int64, len(signerAddrs)) + for i := 0; i < len(signerAddrs); i++ { + accNums[i] = sigs[i].AccountNumber + } fee := stdTx.Fee chainID := ctx.ChainID() // XXX: major hack; need to get ChainID @@ -58,7 +62,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { if chainID == "" { chainID = viper.GetString("chain-id") } - signBytes := StdSignBytes(ctx.ChainID(), sequences, fee, msg) + signBytes := StdSignBytes(ctx.ChainID(), accNums, sequences, fee, msg) // Check sig and nonce and collect signer accounts. var signerAccs = make([]Account, len(signerAddrs)) @@ -117,6 +121,13 @@ func processSig( return nil, sdk.ErrUnknownAddress(addr.String()).Result() } + // Check account number. + accnum := acc.GetAccountNumber() + if accnum != sig.AccountNumber { + return nil, sdk.ErrInvalidSequence( + fmt.Sprintf("Invalid account number. Got %d, expected %d", sig.AccountNumber, accnum)).Result() + } + // Check and increment sequence number. seq := acc.GetSequence() if seq != sig.Sequence { diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index b7f22e5d5..aff80cb59 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -52,15 +52,15 @@ func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), result.Code) } -func newTestTx(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, seqs []int64, fee StdFee) sdk.Tx { - signBytes := StdSignBytes(ctx.ChainID(), seqs, fee, msg) - return newTestTxWithSignBytes(msg, privs, seqs, fee, signBytes) +func newTestTx(ctx sdk.Context, msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee) sdk.Tx { + signBytes := StdSignBytes(ctx.ChainID(), accNums, seqs, fee, msg) + return newTestTxWithSignBytes(msg, privs, accNums, seqs, fee, signBytes) } -func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, seqs []int64, fee StdFee, signBytes []byte) sdk.Tx { +func newTestTxWithSignBytes(msg sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee, signBytes []byte) sdk.Tx { sigs := make([]StdSignature, len(privs)) for i, priv := range privs { - sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: priv.Sign(signBytes), Sequence: seqs[i]} + sigs[i] = StdSignature{PubKey: priv.PubKey(), Signature: priv.Sign(signBytes), AccountNumber: accNums[i], Sequence: seqs[i]} } tx := NewStdTx(msg, fee, sigs) return tx @@ -87,18 +87,18 @@ func TestAnteHandlerSigErrors(t *testing.T) { fee := newStdFee() // test no signatures - privs, seqs := []crypto.PrivKey{}, []int64{} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accNums, seqs := []crypto.PrivKey{}, []int64{}, []int64{} + tx = newTestTx(ctx, msg, privs, accNums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test num sigs dont match GetSigners - privs, seqs = []crypto.PrivKey{priv1}, []int64{0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accNums, seqs = []crypto.PrivKey{priv1}, []int64{0}, []int64{0} + tx = newTestTx(ctx, msg, privs, accNums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test an unrecognized account - privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accNums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{0, 0} + tx = newTestTx(ctx, msg, privs, accNums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnknownAddress) // save the first account, but second is still unrecognized @@ -108,6 +108,61 @@ func TestAnteHandlerSigErrors(t *testing.T) { checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnknownAddress) } +// Test logic around account number checking with one signer and many signers. +func TestAnteHandlerAccountNumbers(t *testing.T) { + // setup + ms, capKey, capKey2 := setupMultiStore() + cdc := wire.NewCodec() + RegisterBaseAccount(cdc) + mapper := NewAccountMapper(cdc, capKey, &BaseAccount{}) + feeCollector := NewFeeCollectionKeeper(cdc, capKey2) + anteHandler := NewAnteHandler(mapper, feeCollector) + ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid"}, false, nil, log.NewNopLogger()) + + // keys and addresses + priv1, addr1 := privAndAddr() + priv2, addr2 := privAndAddr() + + // set the accounts + acc1 := mapper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(newCoins()) + mapper.SetAccount(ctx, acc1) + acc2 := mapper.NewAccountWithAddress(ctx, addr2) + acc2.SetCoins(newCoins()) + mapper.SetAccount(ctx, acc2) + + // msg and signatures + var tx sdk.Tx + msg := newTestMsg(addr1) + fee := newStdFee() + + // test good tx from one signer + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) + checkValidTx(t, anteHandler, ctx, tx) + + // new tx from wrong account number + seqs = []int64{1} + tx = newTestTx(ctx, msg, privs, []int64{1}, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + + // from correct account number + seqs = []int64{1} + tx = newTestTx(ctx, msg, privs, []int64{0}, seqs, fee) + checkValidTx(t, anteHandler, ctx, tx) + + // new tx with another signer and incorrect account numbers + msg = newTestMsg(addr1, addr2) + privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{1, 0}, []int64{2, 0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) + checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) + + // correct account numbers + privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{2, 0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) + checkValidTx(t, anteHandler, ctx, tx) +} + // Test logic around sequence checking with one signer and many signers. func TestAnteHandlerSequences(t *testing.T) { // setup @@ -137,8 +192,8 @@ func TestAnteHandlerSequences(t *testing.T) { fee := newStdFee() // test good tx from one signer - privs, seqs := []crypto.PrivKey{priv1}, []int64{0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // test sending it again fails (replay protection) @@ -146,13 +201,13 @@ func TestAnteHandlerSequences(t *testing.T) { // fix sequence, should pass seqs = []int64{1} - tx = newTestTx(ctx, msg, privs, seqs, fee) + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // new tx with another signer and correct sequences msg = newTestMsg(addr1, addr2) - privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{2, 0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{2, 0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) // replay fails @@ -160,18 +215,18 @@ func TestAnteHandlerSequences(t *testing.T) { // tx from just second signer with incorrect sequence fails msg = newTestMsg(addr2) - privs, seqs = []crypto.PrivKey{priv2}, []int64{0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs = []crypto.PrivKey{priv2}, []int64{1}, []int64{0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidSequence) // fix the sequence and it passes - tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{1}, fee) + tx = newTestTx(ctx, msg, []crypto.PrivKey{priv2}, []int64{1}, []int64{1}, fee) checkValidTx(t, anteHandler, ctx, tx) // another tx from both of them that passes msg = newTestMsg(addr1, addr2) - privs, seqs = []crypto.PrivKey{priv1, priv2}, []int64{3, 2} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs = []crypto.PrivKey{priv1, priv2}, []int64{0, 1}, []int64{3, 2} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) } @@ -196,13 +251,13 @@ func TestAnteHandlerFees(t *testing.T) { // msg and signatures var tx sdk.Tx msg := newTestMsg(addr1) - privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} fee := NewStdFee(100, sdk.Coin{"atom", 150}, ) // signer does not have enough funds to pay the fee - tx = newTestTx(ctx, msg, privs, seqs, fee) + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInsufficientFunds) acc1.SetCoins(sdk.Coins{{"atom", 149}}) @@ -249,8 +304,8 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { fee3.Amount[0].Amount += 100 // test good tx and signBytes - privs, seqs := []crypto.PrivKey{priv1}, []int64{0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) chainID := ctx.ChainID() @@ -259,37 +314,39 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { cases := []struct { chainID string + accnums []int64 seqs []int64 fee StdFee msg sdk.Msg code sdk.CodeType }{ - {chainID2, []int64{1}, fee, msg, codeUnauth}, // test wrong chain_id - {chainID, []int64{2}, fee, msg, codeUnauth}, // test wrong seqs - {chainID, []int64{1, 2}, fee, msg, codeUnauth}, // test wrong seqs - {chainID, []int64{1}, fee, newTestMsg(addr2), codeUnauth}, // test wrong msg - {chainID, []int64{1}, fee2, newTestMsg(addr2), codeUnauth}, // test wrong fee - {chainID, []int64{1}, fee3, newTestMsg(addr2), codeUnauth}, // test wrong fee + {chainID2, []int64{0}, []int64{1}, fee, msg, codeUnauth}, // test wrong chain_id + {chainID, []int64{0}, []int64{2}, fee, msg, codeUnauth}, // test wrong seqs + {chainID, []int64{0}, []int64{1, 2}, fee, msg, codeUnauth}, // test wrong seqs + {chainID, []int64{1}, []int64{1}, fee, msg, codeUnauth}, // test wrong accnum + {chainID, []int64{0}, []int64{1}, fee, newTestMsg(addr2), codeUnauth}, // test wrong msg + {chainID, []int64{0}, []int64{1}, fee2, msg, codeUnauth}, // test wrong fee + {chainID, []int64{0}, []int64{1}, fee3, msg, codeUnauth}, // test wrong fee } privs, seqs = []crypto.PrivKey{priv1}, []int64{1} for _, cs := range cases { tx := newTestTxWithSignBytes( - msg, privs, seqs, fee, - StdSignBytes(cs.chainID, cs.seqs, cs.fee, cs.msg), + msg, privs, accnums, seqs, fee, + StdSignBytes(cs.chainID, cs.accnums, cs.seqs, cs.fee, cs.msg), ) checkInvalidTx(t, anteHandler, ctx, tx, cs.code) } // test wrong signer if public key exist - privs, seqs = []crypto.PrivKey{priv2}, []int64{1} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs = []crypto.PrivKey{priv2}, []int64{0}, []int64{1} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeUnauthorized) // test wrong signer if public doesn't exist msg = newTestMsg(addr2) - privs, seqs = []crypto.PrivKey{priv1}, []int64{0} - tx = newTestTx(ctx, msg, privs, seqs, fee) + privs, accnums, seqs = []crypto.PrivKey{priv1}, []int64{1}, []int64{0} + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) } @@ -320,9 +377,9 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // test good tx and set public key msg := newTestMsg(addr1) - privs, seqs := []crypto.PrivKey{priv1}, []int64{0} + privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0} fee := newStdFee() - tx = newTestTx(ctx, msg, privs, seqs, fee) + tx = newTestTx(ctx, msg, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx) acc1 = mapper.GetAccount(ctx, addr1) @@ -330,7 +387,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // test public key not found msg = newTestMsg(addr2) - tx = newTestTx(ctx, msg, privs, seqs, fee) + tx = newTestTx(ctx, msg, privs, []int64{1}, seqs, fee) sigs := tx.(StdTx).GetSignatures() sigs[0].PubKey = nil checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) @@ -339,7 +396,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { assert.Nil(t, acc2.GetPubKey()) // test invalid signature and public key - tx = newTestTx(ctx, msg, privs, seqs, fee) + tx = newTestTx(ctx, msg, privs, []int64{1}, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, sdk.CodeInvalidPubKey) acc2 = mapper.GetAccount(ctx, addr2) diff --git a/x/auth/client/cli/account.go b/x/auth/client/cli/account.go index e82b07414..a3265a78c 100644 --- a/x/auth/client/cli/account.go +++ b/x/auth/client/cli/account.go @@ -47,7 +47,7 @@ func GetAccountCmd(storeName string, cdc *wire.Codec, decoder auth.AccountDecode // perform query ctx := context.NewCoreContextFromViper() - res, err := ctx.Query(key, storeName) + res, err := ctx.Query(auth.AddressStoreKey(key), storeName) if err != nil { return err } diff --git a/x/auth/client/rest/query.go b/x/auth/client/rest/query.go index bcae59c20..9ccbe8e14 100644 --- a/x/auth/client/rest/query.go +++ b/x/auth/client/rest/query.go @@ -34,7 +34,7 @@ func QueryAccountRequestHandlerFn(storeName string, cdc *wire.Codec, decoder aut return } - res, err := ctx.Query(addr, storeName) + res, err := ctx.Query(auth.AddressStoreKey(addr), storeName) if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(fmt.Sprintf("Could't query account. Error: %s", err.Error()))) diff --git a/x/auth/mapper.go b/x/auth/mapper.go index cdab2480e..b4364f768 100644 --- a/x/auth/mapper.go +++ b/x/auth/mapper.go @@ -9,6 +9,8 @@ import ( crypto "github.com/tendermint/go-crypto" ) +var globalAccountNumberKey = []byte("globalAccountNumber") + // This AccountMapper encodes/decodes accounts using the // go-amino (binary) encoding/decoding library. type AccountMapper struct { @@ -38,13 +40,25 @@ func NewAccountMapper(cdc *wire.Codec, key sdk.StoreKey, proto Account) AccountM func (am AccountMapper) NewAccountWithAddress(ctx sdk.Context, addr sdk.Address) Account { acc := am.clonePrototype() acc.SetAddress(addr) + acc.SetAccountNumber(am.GetNextAccountNumber(ctx)) return acc } +// New Account +func (am AccountMapper) NewAccount(ctx sdk.Context, acc Account) Account { + acc.SetAccountNumber(am.GetNextAccountNumber(ctx)) + return acc +} + +// Turn an address to key used to get it from the account store +func AddressStoreKey(addr sdk.Address) []byte { + return append([]byte("account:"), addr.Bytes()...) +} + // Implements sdk.AccountMapper. func (am AccountMapper) GetAccount(ctx sdk.Context, addr sdk.Address) Account { store := ctx.KVStore(am.key) - bz := store.Get(addr) + bz := store.Get(AddressStoreKey(addr)) if bz == nil { return nil } @@ -57,13 +71,13 @@ func (am AccountMapper) SetAccount(ctx sdk.Context, acc Account) { addr := acc.GetAddress() store := ctx.KVStore(am.key) bz := am.encodeAccount(acc) - store.Set(addr, bz) + store.Set(AddressStoreKey(addr), bz) } // Implements sdk.AccountMapper. func (am AccountMapper) IterateAccounts(ctx sdk.Context, process func(Account) (stop bool)) { store := ctx.KVStore(am.key) - iter := store.Iterator(nil, nil) + iter := sdk.KVStorePrefixIterator(store, []byte("account:")) for { if !iter.Valid() { return @@ -116,6 +130,26 @@ func (am AccountMapper) setSequence(ctx sdk.Context, addr sdk.Address, newSequen return nil } +// Returns and increments the global account number counter +func (am AccountMapper) GetNextAccountNumber(ctx sdk.Context) int64 { + var accNumber int64 + store := ctx.KVStore(am.key) + bz := store.Get(globalAccountNumberKey) + if bz == nil { + accNumber = 0 + } else { + err := am.cdc.UnmarshalBinary(bz, &accNumber) + if err != nil { + panic(err) + } + } + + bz = am.cdc.MustMarshalBinary(accNumber + 1) + store.Set(globalAccountNumberKey, bz) + + return accNumber +} + //---------------------------------------- // misc. diff --git a/x/auth/mock/app.go b/x/auth/mock/app.go index fcd130b44..953008807 100644 --- a/x/auth/mock/app.go +++ b/x/auth/mock/app.go @@ -78,7 +78,9 @@ func (app *App) CompleteSetup(t *testing.T, newKeys []*sdk.KVStoreKey) { func (app *App) InitChainer(ctx sdk.Context, _ abci.RequestInitChain) abci.ResponseInitChain { // load the accounts - for _, acc := range app.GenesisAccounts { + for _, genacc := range app.GenesisAccounts { + acc := app.AccountMapper.NewAccountWithAddress(ctx, genacc.GetAddress()) + acc.SetCoins(genacc.GetCoins()) app.AccountMapper.SetAccount(ctx, acc) } diff --git a/x/auth/mock/auth_app_test.go b/x/auth/mock/auth_app_test.go index 716037d60..bb4d7007b 100644 --- a/x/auth/mock/auth_app_test.go +++ b/x/auth/mock/auth_app_test.go @@ -61,7 +61,7 @@ func TestMsgChangePubKey(t *testing.T) { assert.Equal(t, acc1, res1.(*auth.BaseAccount)) // Run a CheckDeliver - SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, true, priv1) + SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{0}, true, priv1) // Check balances CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 67}}) @@ -77,19 +77,19 @@ func TestMsgChangePubKey(t *testing.T) { acc2 := mapp.AccountMapper.GetAccount(ctxDeliver, addr1) // send a MsgChangePubKey - SignCheckDeliver(t, mapp.BaseApp, changePubKeyMsg, []int64{1}, true, priv1) + SignCheckDeliver(t, mapp.BaseApp, changePubKeyMsg, []int64{0}, []int64{1}, true, priv1) acc2 = mapp.AccountMapper.GetAccount(ctxDeliver, addr1) assert.True(t, priv2.PubKey().Equals(acc2.GetPubKey())) // signing a SendMsg with the old privKey should be an auth error mapp.BeginBlock(abci.RequestBeginBlock{}) - tx := GenTx(sendMsg1, []int64{2}, priv1) + tx := GenTx(sendMsg1, []int64{0}, []int64{2}, priv1) res := mapp.Deliver(tx) assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log) // resigning the tx with the new correct priv key should work - SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{2}, true, priv2) + SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{2}, true, priv2) // Check balances CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 57}}) diff --git a/x/auth/mock/simulate_block.go b/x/auth/mock/simulate_block.go index 72ec8411f..7a77e0f09 100644 --- a/x/auth/mock/simulate_block.go +++ b/x/auth/mock/simulate_block.go @@ -33,7 +33,7 @@ func CheckBalance(t *testing.T, app *App, addr sdk.Address, exp sdk.Coins) { } // generate a signed transaction -func GenTx(msg sdk.Msg, seq []int64, priv ...crypto.PrivKeyEd25519) auth.StdTx { +func GenTx(msg sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) auth.StdTx { // make the transaction free fee := auth.StdFee{ @@ -44,19 +44,27 @@ func GenTx(msg sdk.Msg, seq []int64, priv ...crypto.PrivKeyEd25519) auth.StdTx { sigs := make([]auth.StdSignature, len(priv)) for i, p := range priv { sigs[i] = auth.StdSignature{ - PubKey: p.PubKey(), - Signature: p.Sign(auth.StdSignBytes(chainID, seq, fee, msg)), - Sequence: seq[i], + PubKey: p.PubKey(), + Signature: p.Sign(auth.StdSignBytes(chainID, accnums, seq, fee, msg)), + AccountNumber: accnums[i], + Sequence: seq[i], } } return auth.NewStdTx(msg, fee, sigs) } +// check a transaction result +func SignCheck(t *testing.T, app *baseapp.BaseApp, msg sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) sdk.Result { + tx := GenTx(msg, accnums, seq, priv...) + res := app.Check(tx) + return res +} + // simulate a block -func SignCheckDeliver(t *testing.T, app *baseapp.BaseApp, msg sdk.Msg, seq []int64, expPass bool, priv ...crypto.PrivKeyEd25519) { +func SignCheckDeliver(t *testing.T, app *baseapp.BaseApp, msg sdk.Msg, accnums []int64, seq []int64, expPass bool, priv ...crypto.PrivKeyEd25519) { // Sign the tx - tx := GenTx(msg, seq, priv...) + tx := GenTx(msg, accnums, seq, priv...) // Run a Check res := app.Check(tx) diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index e08f77ee4..5c43a3717 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -85,21 +85,23 @@ func (fee StdFee) Bytes() []byte { // and the Sequence numbers for each signature (prevent // inchain replay and enforce tx ordering per account). type StdSignDoc struct { - ChainID string `json:"chain_id"` - Sequences []int64 `json:"sequences"` - FeeBytes json.RawMessage `json:"fee_bytes"` - MsgBytes json.RawMessage `json:"msg_bytes"` - AltBytes json.RawMessage `json:"alt_bytes"` + ChainID string `json:"chain_id"` + AccountNumbers []int64 `json:"account_numbers"` + Sequences []int64 `json:"sequences"` + FeeBytes []byte `json:"fee_bytes"` + MsgBytes []byte `json:"msg_bytes"` + AltBytes []byte `json:"alt_bytes"` } // StdSignBytes returns the bytes to sign for a transaction. // TODO: change the API to just take a chainID and StdTx ? -func StdSignBytes(chainID string, sequences []int64, fee StdFee, msg sdk.Msg) []byte { - bz, err := msgCdc.MarshalJSON(StdSignDoc{ - ChainID: chainID, - Sequences: sequences, - FeeBytes: json.RawMessage(fee.Bytes()), - MsgBytes: json.RawMessage(msg.GetSignBytes()), +func StdSignBytes(chainID string, accnums []int64, sequences []int64, fee StdFee, msg sdk.Msg) []byte { + bz, err := json.Marshal(StdSignDoc{ + ChainID: chainID, + AccountNumbers: accnums, + Sequences: sequences, + FeeBytes: fee.Bytes(), + MsgBytes: msg.GetSignBytes(), }) if err != nil { panic(err) @@ -111,21 +113,23 @@ func StdSignBytes(chainID string, sequences []int64, fee StdFee, msg sdk.Msg) [] // a Msg with the other requirements for a StdSignDoc before // it is signed. For use in the CLI. type StdSignMsg struct { - ChainID string - Sequences []int64 - Fee StdFee - Msg sdk.Msg + ChainID string + AccountNumbers []int64 + Sequences []int64 + Fee StdFee + Msg sdk.Msg // XXX: Alt } // get message bytes func (msg StdSignMsg) Bytes() []byte { - return StdSignBytes(msg.ChainID, msg.Sequences, msg.Fee, msg.Msg) + return StdSignBytes(msg.ChainID, msg.AccountNumbers, msg.Sequences, msg.Fee, msg.Msg) } // Standard Signature type StdSignature struct { crypto.PubKey `json:"pub_key"` // optional crypto.Signature `json:"signature"` + AccountNumber int64 `json:"account_number"` Sequence int64 `json:"sequence"` } diff --git a/x/bank/app_test.go b/x/bank/app_test.go index 21d7d03a0..d0c112b3d 100644 --- a/x/bank/app_test.go +++ b/x/bank/app_test.go @@ -107,25 +107,25 @@ func TestMsgSendWithAccounts(t *testing.T) { assert.Equal(t, acc, res1.(*auth.BaseAccount)) // Run a CheckDeliver - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{0}, true, priv1) // Check balances mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 57}}) mock.CheckBalance(t, mapp, addr2, sdk.Coins{{"foocoin", 10}}) // Delivering again should cause replay error - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, false, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{0}, false, priv1) // bumping the txnonce number without resigning should be an auth error mapp.BeginBlock(abci.RequestBeginBlock{}) - tx := mock.GenTx(sendMsg1, []int64{0}, priv1) + tx := mock.GenTx(sendMsg1, []int64{0}, []int64{0}, priv1) tx.Signatures[0].Sequence = 1 res := mapp.Deliver(tx) assert.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log) // resigning the tx with the bumped sequence should work - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{1}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{1}, true, priv1) } func TestMsgSendMultipleOut(t *testing.T) { @@ -145,7 +145,7 @@ func TestMsgSendMultipleOut(t *testing.T) { mock.SetGenesis(mapp, accs) // Simulate a Block - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg2, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg2, []int64{0}, []int64{0}, true, priv1) // Check balances mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 32}}) @@ -173,7 +173,7 @@ func TestSengMsgMultipleInOut(t *testing.T) { mock.SetGenesis(mapp, accs) // CheckDeliver - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg3, []int64{0, 0}, true, priv1, priv4) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg3, []int64{0, 2}, []int64{0, 0}, true, priv1, priv4) // Check balances mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 32}}) @@ -194,14 +194,14 @@ func TestMsgSendDependent(t *testing.T) { mock.SetGenesis(mapp, accs) // CheckDeliver - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg1, []int64{0}, []int64{0}, true, priv1) // Check balances mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 32}}) mock.CheckBalance(t, mapp, addr2, sdk.Coins{{"foocoin", 10}}) // Simulate a Block - mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg4, []int64{0}, true, priv2) + mock.SignCheckDeliver(t, mapp.BaseApp, sendMsg4, []int64{1}, []int64{0}, true, priv2) // Check balances mock.CheckBalance(t, mapp, addr1, sdk.Coins{{"foocoin", 42}}) diff --git a/x/bank/client/rest/sendtx.go b/x/bank/client/rest/sendtx.go index 28a294617..2639d2788 100644 --- a/x/bank/client/rest/sendtx.go +++ b/x/bank/client/rest/sendtx.go @@ -27,6 +27,7 @@ type sendBody struct { LocalAccountName string `json:"name"` Password string `json:"password"` ChainID string `json:"chain_id"` + AccountNumber int64 `json:"account_number"` Sequence int64 `json:"sequence"` Gas int64 `json:"gas"` } @@ -91,6 +92,7 @@ func SendRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreCont ctx = ctx.WithGas(m.Gas) // sign + ctx = ctx.WithAccountNumber(m.AccountNumber) ctx = ctx.WithSequence(m.Sequence) txBytes, err := ctx.SignAndBuild(m.LocalAccountName, m.Password, msg, cdc) if err != nil { diff --git a/x/ibc/app_test.go b/x/ibc/app_test.go index 9e4b4bf56..90d426882 100644 --- a/x/ibc/app_test.go +++ b/x/ibc/app_test.go @@ -5,9 +5,9 @@ import ( "github.com/stretchr/testify/assert" - "github.com/cosmos/cosmos-sdk/x/auth/mock" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/auth/mock" "github.com/cosmos/cosmos-sdk/x/bank" abci "github.com/tendermint/abci/types" @@ -70,10 +70,10 @@ func TestIBCMsgs(t *testing.T) { Sequence: 0, } - mock.SignCheckDeliver(t, mapp.BaseApp, transferMsg, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, transferMsg, []int64{0},[]int64{0}, true, priv1) mock.CheckBalance(t, mapp, addr1, emptyCoins) - mock.SignCheckDeliver(t, mapp.BaseApp, transferMsg, []int64{1}, false, priv1) - mock.SignCheckDeliver(t, mapp.BaseApp, receiveMsg, []int64{2}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, transferMsg, []int64{0}, []int64{1}, false, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, receiveMsg, []int64{0}, []int64{2}, true, priv1) mock.CheckBalance(t, mapp, addr1, coins) - mock.SignCheckDeliver(t, mapp.BaseApp, receiveMsg, []int64{3}, false, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, receiveMsg, []int64{0}, []int64{3}, false, priv1) } diff --git a/x/ibc/client/rest/transfer.go b/x/ibc/client/rest/transfer.go index 2b26b9f01..b77a6f5eb 100644 --- a/x/ibc/client/rest/transfer.go +++ b/x/ibc/client/rest/transfer.go @@ -25,6 +25,7 @@ type transferBody struct { LocalAccountName string `json:"name"` Password string `json:"password"` SrcChainID string `json:"src_chain_id"` + AccountNumber int64 `json:"account_number"` Sequence int64 `json:"sequence"` Gas int64 `json:"gas"` } @@ -82,6 +83,7 @@ func TransferRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.Core ctx = ctx.WithGas(m.Gas) // sign + ctx = ctx.WithAccountNumber(m.AccountNumber) ctx = ctx.WithSequence(m.Sequence) txBytes, err := ctx.SignAndBuild(m.LocalAccountName, m.Password, msg, cdc) if err != nil { diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go new file mode 100644 index 000000000..2a99795c6 --- /dev/null +++ b/x/slashing/app_test.go @@ -0,0 +1,111 @@ +package slashing + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth" + "github.com/cosmos/cosmos-sdk/x/auth/mock" + "github.com/cosmos/cosmos-sdk/x/bank" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/x/stake" + abci "github.com/tendermint/abci/types" + crypto "github.com/tendermint/go-crypto" +) + +var ( + priv1 = crypto.GenPrivKeyEd25519() + addr1 = priv1.PubKey().Address() + coins = sdk.Coins{{"foocoin", 10}} +) + +// initialize the mock application for this module +func getMockApp(t *testing.T) (*mock.App, stake.Keeper, Keeper) { + mapp := mock.NewApp() + + RegisterWire(mapp.Cdc) + keyStake := sdk.NewKVStoreKey("stake") + keySlashing := sdk.NewKVStoreKey("slashing") + coinKeeper := bank.NewKeeper(mapp.AccountMapper) + stakeKeeper := stake.NewKeeper(mapp.Cdc, keyStake, coinKeeper, mapp.RegisterCodespace(stake.DefaultCodespace)) + keeper := NewKeeper(mapp.Cdc, keySlashing, stakeKeeper, mapp.RegisterCodespace(DefaultCodespace)) + mapp.Router().AddRoute("stake", stake.NewHandler(stakeKeeper)) + mapp.Router().AddRoute("slashing", NewHandler(keeper)) + + mapp.SetEndBlocker(getEndBlocker(stakeKeeper)) + mapp.SetInitChainer(getInitChainer(mapp, stakeKeeper)) + mapp.CompleteSetup(t, []*sdk.KVStoreKey{keyStake, keySlashing}) + + return mapp, stakeKeeper, keeper +} + +// stake endblocker +func getEndBlocker(keeper stake.Keeper) sdk.EndBlocker { + return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { + validatorUpdates := stake.EndBlocker(ctx, keeper) + return abci.ResponseEndBlock{ + ValidatorUpdates: validatorUpdates, + } + } +} + +// overwrite the mock init chainer +func getInitChainer(mapp *mock.App, keeper stake.Keeper) sdk.InitChainer { + return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { + mapp.InitChainer(ctx, req) + stake.InitGenesis(ctx, keeper, stake.DefaultGenesisState()) + return abci.ResponseInitChain{} + } +} + +func checkValidator(t *testing.T, mapp *mock.App, keeper stake.Keeper, + addr sdk.Address, expFound bool) stake.Validator { + ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{}) + validator, found := keeper.GetValidator(ctxCheck, addr1) + assert.Equal(t, expFound, found) + return validator +} + +func checkValidatorSigningInfo(t *testing.T, mapp *mock.App, keeper Keeper, + addr sdk.Address, expFound bool) ValidatorSigningInfo { + ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{}) + signingInfo, found := keeper.getValidatorSigningInfo(ctxCheck, addr) + assert.Equal(t, expFound, found) + return signingInfo +} + +func TestSlashingMsgs(t *testing.T) { + mapp, stakeKeeper, keeper := getMockApp(t) + + genCoin := sdk.Coin{"steak", 42} + bondCoin := sdk.Coin{"steak", 10} + + acc1 := &auth.BaseAccount{ + Address: addr1, + Coins: sdk.Coins{genCoin}, + } + accs := []auth.Account{acc1} + mock.SetGenesis(mapp, accs) + description := stake.NewDescription("foo_moniker", "", "", "") + createValidatorMsg := stake.NewMsgCreateValidator( + addr1, priv1.PubKey(), bondCoin, description, + ) + mock.SignCheckDeliver(t, mapp.BaseApp, createValidatorMsg, []int64{0}, []int64{0}, true, priv1) + mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin.Minus(bondCoin)}) + mapp.BeginBlock(abci.RequestBeginBlock{}) + + validator := checkValidator(t, mapp, stakeKeeper, addr1, true) + require.Equal(t, addr1, validator.Owner) + require.Equal(t, sdk.Bonded, validator.Status()) + require.True(sdk.RatEq(t, sdk.NewRat(10), validator.PoolShares.Bonded())) + unrevokeMsg := MsgUnrevoke{ValidatorAddr: validator.PubKey.Address()} + + // no signing info yet + checkValidatorSigningInfo(t, mapp, keeper, addr1, false) + + // unrevoke should fail with unknown validator + res := mock.SignCheck(t, mapp.BaseApp, unrevokeMsg, []int64{0}, []int64{1}, priv1) + require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeInvalidValidator), res.Code) +} diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index d558cc04b..d5ae09ef2 100644 --- a/x/slashing/keeper.go +++ b/x/slashing/keeper.go @@ -55,8 +55,12 @@ func (k Keeper) handleValidatorSignature(ctx sdk.Context, pubkey crypto.PubKey, address := pubkey.Address() // Local index, so counts blocks validator *should* have signed - // Will use the 0-value default signing info if not present - signInfo, _ := k.getValidatorSigningInfo(ctx, address) + // Will use the 0-value default signing info if not present, except for start height + signInfo, found := k.getValidatorSigningInfo(ctx, address) + if !found { + // If this validator has never been seen before, construct a new SigningInfo with the correct start height + signInfo = NewValidatorSigningInfo(height, 0, 0, 0) + } index := signInfo.IndexOffset % SignedBlocksWindow signInfo.IndexOffset++ diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index 25ae1686d..1f8f9db07 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -11,6 +11,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/stake" ) +// Test that a validator is slashed correctly +// when we discover evidence of equivocation func TestHandleDoubleSign(t *testing.T) { // initial setup @@ -32,6 +34,8 @@ func TestHandleDoubleSign(t *testing.T) { require.Equal(t, sdk.NewRat(amt).Mul(sdk.NewRat(19).Quo(sdk.NewRat(20))), sk.Validator(ctx, addr).GetPower()) } +// Test a validator through uptime, downtime, revocation, +// unrevocation, starting height reset, and revocation again func TestHandleAbsentValidator(t *testing.T) { // initial setup @@ -129,3 +133,39 @@ func TestHandleAbsentValidator(t *testing.T) { validator, _ = sk.GetValidatorByPubKey(ctx, val) require.Equal(t, sdk.Unbonded, validator.GetStatus()) } + +// Test a new validator entering the validator set +// Ensure that SigningInfo.StartHeight is set correctly +// and that they are not immediately revoked +func TestHandleNewValidator(t *testing.T) { + // initial setup + ctx, ck, sk, keeper := createTestInput(t) + addr, val, amt := addrs[0], pks[0], int64(100) + sh := stake.NewHandler(sk) + got := sh(ctx, newTestMsgCreateValidator(addr, val, amt)) + require.True(t, got.IsOK()) + stake.EndBlocker(ctx, sk) + require.Equal(t, ck.GetCoins(ctx, addr), sdk.Coins{{sk.GetParams(ctx).BondDenom, initCoins - amt}}) + require.Equal(t, sdk.NewRat(amt), sk.Validator(ctx, addr).GetPower()) + + // 1000 first blocks not a validator + ctx = ctx.WithBlockHeight(1001) + + // Now a validator, for two blocks + keeper.handleValidatorSignature(ctx, val, true) + ctx = ctx.WithBlockHeight(1002) + keeper.handleValidatorSignature(ctx, val, false) + + info, found := keeper.getValidatorSigningInfo(ctx, val.Address()) + require.True(t, found) + require.Equal(t, int64(1001), info.StartHeight) + require.Equal(t, int64(2), info.IndexOffset) + require.Equal(t, int64(1), info.SignedBlocksCounter) + require.Equal(t, int64(0), info.JailedUntil) + + // validator should be bonded still, should not have been revoked or slashed + validator, _ := sk.GetValidatorByPubKey(ctx, val) + require.Equal(t, sdk.Bonded, validator.GetStatus()) + pool := sk.GetPool(ctx) + require.Equal(t, int64(100), pool.BondedTokens) +} diff --git a/x/slashing/signing_info.go b/x/slashing/signing_info.go index a2df0505a..acbe1738b 100644 --- a/x/slashing/signing_info.go +++ b/x/slashing/signing_info.go @@ -47,6 +47,16 @@ func (k Keeper) setValidatorSigningBitArray(ctx sdk.Context, address sdk.Address store.Set(GetValidatorSigningBitArrayKey(address, index), bz) } +// Construct a new `ValidatorSigningInfo` struct +func NewValidatorSigningInfo(startHeight int64, indexOffset int64, jailedUntil int64, signedBlocksCounter int64) ValidatorSigningInfo { + return ValidatorSigningInfo{ + StartHeight: startHeight, + IndexOffset: indexOffset, + JailedUntil: jailedUntil, + SignedBlocksCounter: signedBlocksCounter, + } +} + // Signing info for a validator type ValidatorSigningInfo struct { StartHeight int64 `json:"start_height"` // height at which validator was first a candidate OR was unrevoked diff --git a/x/stake/app_test.go b/x/stake/app_test.go index 8e20633a0..940d4db2b 100644 --- a/x/stake/app_test.go +++ b/x/stake/app_test.go @@ -117,7 +117,7 @@ func TestStakeMsgs(t *testing.T) { createValidatorMsg := NewMsgCreateValidator( addr1, priv1.PubKey(), bondCoin, description, ) - mock.SignCheckDeliver(t, mapp.BaseApp, createValidatorMsg, []int64{0}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, createValidatorMsg, []int64{0}, []int64{0}, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin.Minus(bondCoin)}) mapp.BeginBlock(abci.RequestBeginBlock{}) @@ -134,7 +134,7 @@ func TestStakeMsgs(t *testing.T) { description = NewDescription("bar_moniker", "", "", "") editValidatorMsg := NewMsgEditValidator(addr1, description) - mock.SignCheckDeliver(t, mapp.BaseApp, editValidatorMsg, []int64{1}, true, priv1) + mock.SignCheckDeliver(t, mapp.BaseApp, editValidatorMsg, []int64{0}, []int64{1}, true, priv1) validator = checkValidator(t, mapp, keeper, addr1, true) require.Equal(t, description, validator.Description) @@ -143,7 +143,7 @@ func TestStakeMsgs(t *testing.T) { mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin}) delegateMsg := NewMsgDelegate(addr2, addr1, bondCoin) - mock.SignCheckDeliver(t, mapp.BaseApp, delegateMsg, []int64{0}, true, priv2) + mock.SignCheckDeliver(t, mapp.BaseApp, delegateMsg, []int64{1}, []int64{0}, true, priv2) mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin.Minus(bondCoin)}) checkDelegation(t, mapp, keeper, addr2, addr1, true, sdk.NewRat(10)) @@ -151,7 +151,7 @@ func TestStakeMsgs(t *testing.T) { // Unbond unbondMsg := NewMsgUnbond(addr2, addr1, "MAX") - mock.SignCheckDeliver(t, mapp.BaseApp, unbondMsg, []int64{1}, true, priv2) + mock.SignCheckDeliver(t, mapp.BaseApp, unbondMsg, []int64{1}, []int64{1}, true, priv2) mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin}) checkDelegation(t, mapp, keeper, addr2, addr1, false, sdk.Rat{}) } diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index 19eca4c44..77a6540ee 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -39,6 +39,7 @@ type editDelegationsBody struct { LocalAccountName string `json:"name"` Password string `json:"password"` ChainID string `json:"chain_id"` + AccountNumber int64 `json:"account_number"` Sequence int64 `json:"sequence"` Gas int64 `json:"gas"` Delegate []msgDelegateInput `json:"delegate"` @@ -129,6 +130,7 @@ func editDelegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx conte signedTxs := make([][]byte, len(messages[:])) for i, msg := range messages { // increment sequence for each message + ctx = ctx.WithAccountNumber(m.AccountNumber) ctx = ctx.WithSequence(m.Sequence) m.Sequence++