Merge PR #1492: Improve Module Test Coverage

* Merge pull request #1492: Improve Module Test Coverage
* Revert renaming of SignCheckDeliver [#1492]
* Remove named fields from stake unit tests & fix comments [#1492]
* update for tmlibs->tendermint/libs
* Remove tmlibs dependency

Gopkg.lock

@@ -137,6 +137,7 @@
     ".",
     "hcl/ast",
     "hcl/parser",
+    "hcl/printer",
     "hcl/scanner",
     "hcl/strconv",
     "hcl/token",
@@ -433,7 +434,7 @@
     "netutil",
     "trace"
   ]
-  revision = "87b3feba568e144938625fc5d80ec92566c1a8fe"
+  revision = "ed29d75add3d7c4bf7ca65aac0c6df3d1420216f"
 
 [[projects]]
   branch = "master"

docs/core/app5.md

@@ -59,7 +59,7 @@ func newApp(logger log.Logger, db dbm.DB) abci.Application {
 
 Note we utilize the popular [cobra library](https://github.com/spf13/cobra) 
 for the CLI, in concert with the [viper library](https://github.com/spf13/library) 
-for managing configuration. See our [cli library](https://github.com/tendermint/tmlibs/blob/master/cli/setup.go)
+for managing configuration. See our [cli library](https://github.com/tendermint/blob/master/tmlibs/cli/setup.go)
 for more details.
 
 TODO: compile and run the binary

examples/democoin/app/app_test.go

@@ -4,14 +4,12 @@ import (
 	"os"
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/cosmos/cosmos-sdk/examples/democoin/types"
 	"github.com/cosmos/cosmos-sdk/examples/democoin/x/cool"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/wire"
 	"github.com/cosmos/cosmos-sdk/x/auth"
-
+	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 	dbm "github.com/tendermint/tendermint/libs/db"

examples/democoin/x/cool/app_test.go

@@ -3,15 +3,13 @@ package cool
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/crypto"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth"
-	"github.com/cosmos/cosmos-sdk/x/auth/mock"
 	bank "github.com/cosmos/cosmos-sdk/x/bank"
+	"github.com/cosmos/cosmos-sdk/x/mock"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto"
 )
 
 var (

examples/democoin/x/pow/app_test.go

@@ -7,8 +7,8 @@ import (
 
 	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/cosmos/cosmos-sdk/x/mock"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"

x/auth/mock/app.go

@@ -1,110 +0,0 @@
-package mock
-
-import (
-	"os"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/crypto"
-	dbm "github.com/tendermint/tendermint/libs/db"
-	"github.com/tendermint/tendermint/libs/log"
-
-	bam "github.com/cosmos/cosmos-sdk/baseapp"
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/wire"
-	"github.com/cosmos/cosmos-sdk/x/auth"
-)
-
-// Extended ABCI application
-type App struct {
-	*bam.BaseApp
-	Cdc        *wire.Codec // public since the codec is passed into the module anyways.
-	KeyMain    *sdk.KVStoreKey
-	KeyAccount *sdk.KVStoreKey
-
-	// TODO: Abstract this out from not needing to be auth specifically
-	AccountMapper       auth.AccountMapper
-	FeeCollectionKeeper auth.FeeCollectionKeeper
-
-	GenesisAccounts []auth.Account
-}
-
-// partially construct a new app on the memstore for module and genesis testing
-func NewApp() *App {
-	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app")
-	db := dbm.NewMemDB()
-
-	// create the cdc with some standard codecs
-	cdc := wire.NewCodec()
-	sdk.RegisterWire(cdc)
-	wire.RegisterCrypto(cdc)
-	auth.RegisterWire(cdc)
-
-	// create your application object
-	app := &App{
-		BaseApp:    bam.NewBaseApp("mock", cdc, logger, db),
-		Cdc:        cdc,
-		KeyMain:    sdk.NewKVStoreKey("main"),
-		KeyAccount: sdk.NewKVStoreKey("acc"),
-	}
-
-	// define the accountMapper
-	app.AccountMapper = auth.NewAccountMapper(
-		app.Cdc,
-		app.KeyAccount,      // target store
-		&auth.BaseAccount{}, // prototype
-	)
-
-	// initialize the app, the chainers and blockers can be overwritten before calling complete setup
-	app.SetInitChainer(app.InitChainer)
-
-	app.SetAnteHandler(auth.NewAnteHandler(app.AccountMapper, app.FeeCollectionKeeper))
-
-	return app
-}
-
-// complete the application setup after the routes have been registered
-func (app *App) CompleteSetup(newKeys []*sdk.KVStoreKey) error {
-	newKeys = append(newKeys, app.KeyMain)
-	newKeys = append(newKeys, app.KeyAccount)
-	app.MountStoresIAVL(newKeys...)
-	err := app.LoadLatestVersion(app.KeyMain)
-	return err
-}
-
-// custom logic for initialization
-func (app *App) InitChainer(ctx sdk.Context, _ abci.RequestInitChain) abci.ResponseInitChain {
-
-	// load the accounts
-	for _, genacc := range app.GenesisAccounts {
-		acc := app.AccountMapper.NewAccountWithAddress(ctx, genacc.GetAddress())
-		err := acc.SetCoins(genacc.GetCoins())
-		if err != nil {
-			// TODO: Handle with #870
-			panic(err)
-		}
-		app.AccountMapper.SetAccount(ctx, acc)
-	}
-
-	return abci.ResponseInitChain{}
-}
-
-// Generate genesis accounts loaded with coins, and returns their addresses, pubkeys, and privkeys
-func CreateGenAccounts(numAccs int64, genCoins sdk.Coins) (genAccs []auth.Account, addrs []sdk.Address, pubKeys []crypto.PubKey, privKeys []crypto.PrivKey) {
-	for i := int64(0); i < numAccs; i++ {
-		privKey := crypto.GenPrivKeyEd25519()
-		pubKey := privKey.PubKey()
-		addr := pubKey.Address()
-
-		genAcc := &auth.BaseAccount{
-			Address: addr,
-			Coins:   genCoins,
-		}
-
-		genAccs = append(genAccs, genAcc)
-		privKeys = append(privKeys, privKey)
-		pubKeys = append(pubKeys, pubKey)
-		addrs = append(addrs, addr)
-	}
-
-	return
-}

x/auth/mock/auth_app_test.go

@@ -1,92 +0,0 @@
-package mock
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
-
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/auth"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/crypto"
-)
-
-// A mock transaction that has a validation which can fail.
-type testMsg struct {
-	signers     []sdk.Address
-	positiveNum int64
-}
-
-// TODO: Clean this up, make it public
-const msgType = "testMsg"
-
-func (tx testMsg) Type() string                       { return msgType }
-func (tx testMsg) GetMsg() sdk.Msg                    { return tx }
-func (tx testMsg) GetMemo() string                    { return "" }
-func (tx testMsg) GetSignBytes() []byte               { return nil }
-func (tx testMsg) GetSigners() []sdk.Address          { return tx.signers }
-func (tx testMsg) GetSignatures() []auth.StdSignature { return nil }
-func (tx testMsg) ValidateBasic() sdk.Error {
-	if tx.positiveNum >= 0 {
-		return nil
-	}
-	return sdk.ErrTxDecode("positiveNum should be a non-negative integer.")
-}
-
-// test auth module messages
-
-var (
-	priv1 = crypto.GenPrivKeyEd25519()
-	addr1 = priv1.PubKey().Address()
-	priv2 = crypto.GenPrivKeyEd25519()
-	addr2 = priv2.PubKey().Address()
-
-	coins    = sdk.Coins{sdk.NewCoin("foocoin", 10)}
-	testMsg1 = testMsg{signers: []sdk.Address{addr1}, positiveNum: 1}
-)
-
-// initialize the mock application for this module
-func getMockApp(t *testing.T) *App {
-	mapp := NewApp()
-
-	mapp.Router().AddRoute(msgType, func(ctx sdk.Context, msg sdk.Msg) (res sdk.Result) { return })
-	require.NoError(t, mapp.CompleteSetup([]*sdk.KVStoreKey{}))
-	return mapp
-}
-
-func TestMsgPrivKeys(t *testing.T) {
-	mapp := getMockApp(t)
-	mapp.Cdc.RegisterConcrete(testMsg{}, "mock/testMsg", nil)
-
-	// Construct some genesis bytes to reflect basecoin/types/AppAccount
-	// Give 77 foocoin to the first key
-	coins := sdk.Coins{sdk.NewCoin("foocoin", 77)}
-	acc1 := &auth.BaseAccount{
-		Address: addr1,
-		Coins:   coins,
-	}
-	accs := []auth.Account{acc1}
-
-	// Construct genesis state
-	SetGenesis(mapp, accs)
-
-	// A checkTx context (true)
-	ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{})
-	res1 := mapp.AccountMapper.GetAccount(ctxCheck, addr1)
-	require.Equal(t, acc1, res1.(*auth.BaseAccount))
-
-	// Run a CheckDeliver
-	SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{testMsg1}, []int64{0}, []int64{0}, true, priv1)
-
-	// signing a SendMsg with the wrong privKey should be an auth error
-	mapp.BeginBlock(abci.RequestBeginBlock{})
-	tx := GenTx([]sdk.Msg{testMsg1}, []int64{0}, []int64{1}, priv2)
-	res := mapp.Deliver(tx)
-	require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log)
-
-	// resigning the tx with the correct priv key should still work
-	res = SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{testMsg1}, []int64{0}, []int64{1}, true, priv1)
-
-	require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOK), res.Code, res.Log)
-}

x/auth/mock/simulate_block.go

@@ -1,110 +0,0 @@
-package mock
-
-import (
-	"testing"
-
-	"github.com/cosmos/cosmos-sdk/baseapp"
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/auth"
-	"github.com/stretchr/testify/require"
-	"github.com/tendermint/tendermint/crypto"
-
-	abci "github.com/tendermint/tendermint/abci/types"
-)
-
-var chainID = "" // TODO
-
-// set the mock app genesis
-func SetGenesis(app *App, accs []auth.Account) {
-
-	// pass the accounts in via the application (lazy) instead of through RequestInitChain
-	app.GenesisAccounts = accs
-
-	app.InitChain(abci.RequestInitChain{})
-	app.Commit()
-}
-
-// check an account balance
-func CheckBalance(t *testing.T, app *App, addr sdk.Address, exp sdk.Coins) {
-	ctxCheck := app.BaseApp.NewContext(true, abci.Header{})
-	res := app.AccountMapper.GetAccount(ctxCheck, addr)
-	require.Equal(t, exp, res.GetCoins())
-}
-
-// generate a signed transaction
-func GenTx(msgs []sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) auth.StdTx {
-
-	// make the transaction free
-	fee := auth.StdFee{
-		sdk.Coins{sdk.NewCoin("foocoin", 0)},
-		100000,
-	}
-
-	sigs := make([]auth.StdSignature, len(priv))
-	memo := "testmemotestmemo"
-	for i, p := range priv {
-		sig, err := p.Sign(auth.StdSignBytes(chainID, accnums[i], seq[i], fee, msgs, memo))
-		if err != nil {
-			panic(err)
-		}
-		sigs[i] = auth.StdSignature{
-			PubKey:        p.PubKey(),
-			Signature:     sig,
-			AccountNumber: accnums[i],
-			Sequence:      seq[i],
-		}
-	}
-	return auth.NewStdTx(msgs, fee, sigs, memo)
-}
-
-// generate a set of signed transactions a msg, that differ only by having the
-// sequence numbers incremented between every transaction.
-func GenSequenceOfTxs(msgs []sdk.Msg, accnums []int64, initSeqNums []int64, numToGenerate int, priv ...crypto.PrivKeyEd25519) []auth.StdTx {
-	txs := make([]auth.StdTx, numToGenerate, numToGenerate)
-	for i := 0; i < numToGenerate; i++ {
-		txs[i] = GenTx(msgs, accnums, initSeqNums, priv...)
-		incrementAllSequenceNumbers(initSeqNums)
-	}
-	return txs
-}
-
-func incrementAllSequenceNumbers(initSeqNums []int64) {
-	for i := 0; i < len(initSeqNums); i++ {
-		initSeqNums[i]++
-	}
-}
-
-// check a transaction result
-func SignCheck(app *baseapp.BaseApp, msgs []sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKeyEd25519) sdk.Result {
-	tx := GenTx(msgs, accnums, seq, priv...)
-	res := app.Check(tx)
-	return res
-}
-
-// simulate a block
-func SignCheckDeliver(t *testing.T, app *baseapp.BaseApp, msgs []sdk.Msg, accnums []int64, seq []int64, expPass bool, priv ...crypto.PrivKeyEd25519) sdk.Result {
-
-	// Sign the tx
-	tx := GenTx(msgs, accnums, seq, priv...)
-
-	// Run a Check
-	res := app.Check(tx)
-	if expPass {
-		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)
-	} else {
-		require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log)
-	}
-
-	// Simulate a Block
-	app.BeginBlock(abci.RequestBeginBlock{})
-	res = app.Deliver(tx)
-	if expPass {
-		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)
-	} else {
-		require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log)
-	}
-	app.EndBlock(abci.RequestEndBlock{})
-
-	app.Commit()
-	return res
-}

x/bank/app_test.go

@@ -7,7 +7,7 @@ import (
 
 	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/mock"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"

x/bank/bench_test.go

@@ -5,7 +5,7 @@ import (
 
 	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/mock"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 )

x/gov/msgs_test.go

@@ -6,7 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/auth/mock"
+	"github.com/cosmos/cosmos-sdk/x/mock"
 )
 
 var (

x/gov/test_common.go

@@ -12,13 +12,13 @@ import (
 	"github.com/tendermint/tendermint/crypto"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/auth/mock"
 	"github.com/cosmos/cosmos-sdk/x/bank"
+	"github.com/cosmos/cosmos-sdk/x/mock"
 	"github.com/cosmos/cosmos-sdk/x/stake"
 )
 
 // initialize the mock application for this module
-func getMockApp(t *testing.T, numGenAccs int64) (*mock.App, Keeper, stake.Keeper, []sdk.Address, []crypto.PubKey, []crypto.PrivKey) {
+func getMockApp(t *testing.T, numGenAccs int) (*mock.App, Keeper, stake.Keeper, []sdk.Address, []crypto.PubKey, []crypto.PrivKey) {
 	mapp := mock.NewApp()
 
 	stake.RegisterWire(mapp.Cdc)

x/ibc/app_test.go

@@ -7,8 +7,8 @@ import (
 
 	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/cosmos/cosmos-sdk/x/mock"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"

x/mock/app.go

@@ -0,0 +1,168 @@
+package mock
+
+import (
+	"os"
+
+	bam "github.com/cosmos/cosmos-sdk/baseapp"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/wire"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto"
+	dbm "github.com/tendermint/tendermint/libs/db"
+	"github.com/tendermint/tendermint/libs/log"
+)
+
+const chainID = ""
+
+// App extends an ABCI application.
+type App struct {
+	*bam.BaseApp
+	Cdc        *wire.Codec // Cdc is public since the codec is passed into the module anyways
+	KeyMain    *sdk.KVStoreKey
+	KeyAccount *sdk.KVStoreKey
+
+	// TODO: Abstract this out from not needing to be auth specifically
+	AccountMapper       auth.AccountMapper
+	FeeCollectionKeeper auth.FeeCollectionKeeper
+
+	GenesisAccounts []auth.Account
+}
+
+// NewApp partially constructs a new app on the memstore for module and genesis
+// testing.
+func NewApp() *App {
+	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "sdk/app")
+	db := dbm.NewMemDB()
+
+	// Create the cdc with some standard codecs
+	cdc := wire.NewCodec()
+	sdk.RegisterWire(cdc)
+	wire.RegisterCrypto(cdc)
+	auth.RegisterWire(cdc)
+
+	// Create your application object
+	app := &App{
+		BaseApp:    bam.NewBaseApp("mock", cdc, logger, db),
+		Cdc:        cdc,
+		KeyMain:    sdk.NewKVStoreKey("main"),
+		KeyAccount: sdk.NewKVStoreKey("acc"),
+	}
+
+	// Define the accountMapper
+	app.AccountMapper = auth.NewAccountMapper(
+		app.Cdc,
+		app.KeyAccount,
+		&auth.BaseAccount{},
+	)
+
+	// Initialize the app. The chainers and blockers can be overwritten before
+	// calling complete setup.
+	app.SetInitChainer(app.InitChainer)
+	app.SetAnteHandler(auth.NewAnteHandler(app.AccountMapper, app.FeeCollectionKeeper))
+
+	return app
+}
+
+// CompleteSetup completes the application setup after the routes have been
+// registered.
+func (app *App) CompleteSetup(newKeys []*sdk.KVStoreKey) error {
+	newKeys = append(newKeys, app.KeyMain)
+	newKeys = append(newKeys, app.KeyAccount)
+
+	app.MountStoresIAVL(newKeys...)
+	err := app.LoadLatestVersion(app.KeyMain)
+
+	return err
+}
+
+// InitChainer performs custom logic for initialization.
+func (app *App) InitChainer(ctx sdk.Context, _ abci.RequestInitChain) abci.ResponseInitChain {
+	// Load the genesis accounts
+	for _, genacc := range app.GenesisAccounts {
+		acc := app.AccountMapper.NewAccountWithAddress(ctx, genacc.GetAddress())
+		acc.SetCoins(genacc.GetCoins())
+		app.AccountMapper.SetAccount(ctx, acc)
+	}
+
+	return abci.ResponseInitChain{}
+}
+
+// CreateGenAccounts generates genesis accounts loaded with coins, and returns
+// their addresses, pubkeys, and privkeys.
+func CreateGenAccounts(numAccs int, genCoins sdk.Coins) (genAccs []auth.Account, addrs []sdk.Address, pubKeys []crypto.PubKey, privKeys []crypto.PrivKey) {
+	for i := 0; i < numAccs; i++ {
+		privKey := crypto.GenPrivKeyEd25519()
+		pubKey := privKey.PubKey()
+		addr := pubKey.Address()
+
+		genAcc := &auth.BaseAccount{
+			Address: addr,
+			Coins:   genCoins,
+		}
+
+		genAccs = append(genAccs, genAcc)
+		privKeys = append(privKeys, privKey)
+		pubKeys = append(pubKeys, pubKey)
+		addrs = append(addrs, addr)
+	}
+
+	return
+}
+
+// SetGenesis sets the mock app genesis accounts.
+func SetGenesis(app *App, accs []auth.Account) {
+	// Pass the accounts in via the application (lazy) instead of through
+	// RequestInitChain.
+	app.GenesisAccounts = accs
+
+	app.InitChain(abci.RequestInitChain{})
+	app.Commit()
+}
+
+// GenTx generates a signed mock transaction.
+func GenTx(msgs []sdk.Msg, accnums []int64, seq []int64, priv ...crypto.PrivKey) auth.StdTx {
+	// Make the transaction free
+	fee := auth.StdFee{
+		sdk.Coins{sdk.NewCoin("foocoin", 0)},
+		100000,
+	}
+
+	sigs := make([]auth.StdSignature, len(priv))
+	memo := "testmemotestmemo"
+
+	for i, p := range priv {
+		sig, err := p.Sign(auth.StdSignBytes(chainID, accnums[i], seq[i], fee, msgs, memo))
+		if err != nil {
+			panic(err)
+		}
+
+		sigs[i] = auth.StdSignature{
+			PubKey:        p.PubKey(),
+			Signature:     sig,
+			AccountNumber: accnums[i],
+			Sequence:      seq[i],
+		}
+	}
+
+	return auth.NewStdTx(msgs, fee, sigs, memo)
+}
+
+// GenSequenceOfTxs generates a set of signed transactions of messages, such
+// that they differ only by having the sequence numbers incremented between
+// every transaction.
+func GenSequenceOfTxs(msgs []sdk.Msg, accnums []int64, initSeqNums []int64, numToGenerate int, priv ...crypto.PrivKey) []auth.StdTx {
+	txs := make([]auth.StdTx, numToGenerate, numToGenerate)
+	for i := 0; i < numToGenerate; i++ {
+		txs[i] = GenTx(msgs, accnums, initSeqNums, priv...)
+		incrementAllSequenceNumbers(initSeqNums)
+	}
+
+	return txs
+}
+
+func incrementAllSequenceNumbers(initSeqNums []int64) {
+	for i := 0; i < len(initSeqNums); i++ {
+		initSeqNums[i]++
+	}
+}

x/mock/app_test.go

@@ -0,0 +1,102 @@
+package mock
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+const msgType = "testMsg"
+
+var (
+	numAccts                       = 2
+	genCoins                       = sdk.Coins{sdk.NewCoin("foocoin", 77)}
+	accs, addrs, pubKeys, privKeys = CreateGenAccounts(numAccts, genCoins)
+)
+
+// testMsg is a mock transaction that has a validation which can fail.
+type testMsg struct {
+	signers     []sdk.Address
+	positiveNum int64
+}
+
+func (tx testMsg) Type() string                       { return msgType }
+func (tx testMsg) GetMsg() sdk.Msg                    { return tx }
+func (tx testMsg) GetMemo() string                    { return "" }
+func (tx testMsg) GetSignBytes() []byte               { return nil }
+func (tx testMsg) GetSigners() []sdk.Address          { return tx.signers }
+func (tx testMsg) GetSignatures() []auth.StdSignature { return nil }
+func (tx testMsg) ValidateBasic() sdk.Error {
+	if tx.positiveNum >= 0 {
+		return nil
+	}
+	return sdk.ErrTxDecode("positiveNum should be a non-negative integer.")
+}
+
+// getMockApp returns an initialized mock application.
+func getMockApp(t *testing.T) *App {
+	mApp := NewApp()
+
+	mApp.Router().AddRoute(msgType, func(ctx sdk.Context, msg sdk.Msg) (res sdk.Result) { return })
+	require.NoError(t, mApp.CompleteSetup([]*sdk.KVStoreKey{}))
+
+	return mApp
+}
+
+func TestCheckAndDeliverGenTx(t *testing.T) {
+	mApp := getMockApp(t)
+	mApp.Cdc.RegisterConcrete(testMsg{}, "mock/testMsg", nil)
+
+	SetGenesis(mApp, accs)
+	ctxCheck := mApp.BaseApp.NewContext(true, abci.Header{})
+
+	msg := testMsg{signers: []sdk.Address{addrs[0]}, positiveNum: 1}
+
+	acct := mApp.AccountMapper.GetAccount(ctxCheck, addrs[0])
+	require.Equal(t, accs[0], acct.(*auth.BaseAccount))
+
+	SignCheckDeliver(
+		t, mApp.BaseApp, []sdk.Msg{msg},
+		[]int64{accs[0].GetAccountNumber()}, []int64{accs[0].GetSequence()},
+		true, privKeys[0],
+	)
+
+	// Signing a tx with the wrong privKey should result in an auth error
+	res := SignCheckDeliver(
+		t, mApp.BaseApp, []sdk.Msg{msg},
+		[]int64{accs[1].GetAccountNumber()}, []int64{accs[1].GetSequence() + 1},
+		false, privKeys[1],
+	)
+	require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnauthorized), res.Code, res.Log)
+
+	// Resigning the tx with the correct privKey should result in an OK result
+	SignCheckDeliver(
+		t, mApp.BaseApp, []sdk.Msg{msg},
+		[]int64{accs[0].GetAccountNumber()}, []int64{accs[0].GetSequence() + 1},
+		true, privKeys[0],
+	)
+}
+
+func TestCheckGenTx(t *testing.T) {
+	mApp := getMockApp(t)
+	mApp.Cdc.RegisterConcrete(testMsg{}, "mock/testMsg", nil)
+
+	SetGenesis(mApp, accs)
+
+	msg1 := testMsg{signers: []sdk.Address{addrs[0]}, positiveNum: 1}
+	CheckGenTx(
+		t, mApp.BaseApp, []sdk.Msg{msg1},
+		[]int64{accs[0].GetAccountNumber()}, []int64{accs[0].GetSequence()},
+		true, privKeys[0],
+	)
+
+	msg2 := testMsg{signers: []sdk.Address{addrs[0]}, positiveNum: -1}
+	CheckGenTx(
+		t, mApp.BaseApp, []sdk.Msg{msg2},
+		[]int64{accs[0].GetAccountNumber()}, []int64{accs[0].GetSequence()},
+		false, privKeys[0],
+	)
+}

x/mock/test_utils.go

@@ -0,0 +1,71 @@
+package mock
+
+import (
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/baseapp"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/crypto"
+)
+
+// CheckBalance checks the balance of an account.
+func CheckBalance(t *testing.T, app *App, addr sdk.Address, exp sdk.Coins) {
+	ctxCheck := app.BaseApp.NewContext(true, abci.Header{})
+	res := app.AccountMapper.GetAccount(ctxCheck, addr)
+
+	require.Equal(t, exp, res.GetCoins())
+}
+
+// CheckGenTx checks a generated signed transaction. The result of the check is
+// compared against the parameter 'expPass'. A test assertion is made using the
+// parameter 'expPass' against the result. A corresponding result is returned.
+func CheckGenTx(
+	t *testing.T, app *baseapp.BaseApp, msgs []sdk.Msg, accNums []int64,
+	seq []int64, expPass bool, priv ...crypto.PrivKey,
+) sdk.Result {
+	tx := GenTx(msgs, accNums, seq, priv...)
+	res := app.Check(tx)
+
+	if expPass {
+		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)
+	} else {
+		require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log)
+	}
+
+	return res
+}
+
+// SignCheckDeliver checks a generated signed transaction and simulates a
+// block commitment with the given transaction. A test assertion is made using
+// the parameter 'expPass' against the result. A corresponding result is
+// returned.
+func SignCheckDeliver(
+	t *testing.T, app *baseapp.BaseApp, msgs []sdk.Msg, accNums []int64,
+	seq []int64, expPass bool, priv ...crypto.PrivKey,
+) sdk.Result {
+	tx := GenTx(msgs, accNums, seq, priv...)
+	res := app.Check(tx)
+
+	if expPass {
+		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)
+	} else {
+		require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log)
+	}
+
+	// Simulate a sending a transaction and committing a block
+	app.BeginBlock(abci.RequestBeginBlock{})
+	res = app.Deliver(tx)
+
+	if expPass {
+		require.Equal(t, sdk.ABCICodeOK, res.Code, res.Log)
+	} else {
+		require.NotEqual(t, sdk.ABCICodeOK, res.Code, res.Log)
+	}
+
+	app.EndBlock(abci.RequestEndBlock{})
+	app.Commit()
+
+	return res
+}

x/slashing/app_test.go

@@ -5,11 +5,10 @@ import (
 
 	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/require"
+	"github.com/cosmos/cosmos-sdk/x/mock"
-
 	"github.com/cosmos/cosmos-sdk/x/stake"
+	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 )
@@ -103,10 +102,9 @@ func TestSlashingMsgs(t *testing.T) {
 	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 validator not revoked
+	// unrevoke should fail with unknown validator
-	res := mock.SignCheck(mapp.BaseApp, []sdk.Msg{unrevokeMsg}, []int64{0}, []int64{1}, priv1)
+	res := mock.CheckGenTx(t, mapp.BaseApp, []sdk.Msg{unrevokeMsg}, []int64{0}, []int64{1}, false, priv1)
 	require.Equal(t, sdk.ToABCICode(DefaultCodespace, CodeValidatorNotRevoked), res.Code)
 }

x/stake/app_test.go

@@ -5,11 +5,9 @@ import (
 
 	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/cosmos/cosmos-sdk/x/mock"
 	"github.com/stretchr/testify/require"
-
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 )
@@ -22,81 +20,86 @@ var (
 	addr3 = crypto.GenPrivKeyEd25519().PubKey().Address()
 	priv4 = crypto.GenPrivKeyEd25519()
 	addr4 = priv4.PubKey().Address()
-	coins = sdk.Coins{{"foocoin", sdk.NewInt(10)}}
+	coins = sdk.NewCoin("foocoin", 10)
-	fee   = auth.StdFee{
+	fee   = auth.StdFee{sdk.Coins{sdk.NewCoin("foocoin", 0)}, 100000}
-		sdk.Coins{{"foocoin", sdk.NewInt(0)}},
-		100000,
-	}
 )
 
-// initialize the mock application for this module
+// getMockApp returns an initialized mock application for this module.
 func getMockApp(t *testing.T) (*mock.App, Keeper) {
-	mapp := mock.NewApp()
+	mApp := mock.NewApp()
+
+	RegisterWire(mApp.Cdc)
 
-	RegisterWire(mapp.Cdc)
 	keyStake := sdk.NewKVStoreKey("stake")
-	coinKeeper := bank.NewKeeper(mapp.AccountMapper)
+	coinKeeper := bank.NewKeeper(mApp.AccountMapper)
-	keeper := NewKeeper(mapp.Cdc, keyStake, coinKeeper, mapp.RegisterCodespace(DefaultCodespace))
+	keeper := NewKeeper(mApp.Cdc, keyStake, coinKeeper, mApp.RegisterCodespace(DefaultCodespace))
-	mapp.Router().AddRoute("stake", NewHandler(keeper))
 
-	mapp.SetEndBlocker(getEndBlocker(keeper))
+	mApp.Router().AddRoute("stake", NewHandler(keeper))
-	mapp.SetInitChainer(getInitChainer(mapp, keeper))
+	mApp.SetEndBlocker(getEndBlocker(keeper))
+	mApp.SetInitChainer(getInitChainer(mApp, keeper))
 
-	require.NoError(t, mapp.CompleteSetup([]*sdk.KVStoreKey{keyStake}))
+	require.NoError(t, mApp.CompleteSetup([]*sdk.KVStoreKey{keyStake}))
-	return mapp, keeper
+	return mApp, keeper
 }
 
-// stake endblocker
+// getEndBlocker returns a stake endblocker.
 func getEndBlocker(keeper Keeper) sdk.EndBlocker {
 	return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock {
 		validatorUpdates := EndBlocker(ctx, keeper)
+
 		return abci.ResponseEndBlock{
 			ValidatorUpdates: validatorUpdates,
 		}
 	}
 }
 
-// overwrite the mock init chainer
+// getInitChainer initializes the chainer of the mock app and sets the genesis
+// state. It returns an empty ResponseInitChain.
 func getInitChainer(mapp *mock.App, keeper Keeper) sdk.InitChainer {
 	return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 		mapp.InitChainer(ctx, req)
+
 		stakeGenesis := DefaultGenesisState()
 		stakeGenesis.Pool.LooseTokens = 100000
+
 		InitGenesis(ctx, keeper, stakeGenesis)
 
 		return abci.ResponseInitChain{}
 	}
 }
 
-//__________________________________________________________________________________________
+func checkValidator(
-
+	t *testing.T, mapp *mock.App, keeper Keeper,
-func checkValidator(t *testing.T, mapp *mock.App, keeper Keeper,
+	addr sdk.Address, expFound bool,
-	addr sdk.Address, expFound bool) Validator {
+) Validator {
-
 	ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{})
 	validator, found := keeper.GetValidator(ctxCheck, addr1)
+
 	require.Equal(t, expFound, found)
 	return validator
 }
 
-func checkDelegation(t *testing.T, mapp *mock.App, keeper Keeper, delegatorAddr,
+func checkDelegation(
-	validatorAddr sdk.Address, expFound bool, expShares sdk.Rat) {
+	t *testing.T, mapp *mock.App, keeper Keeper, delegatorAddr,
-
+	validatorAddr sdk.Address, expFound bool, expShares sdk.Rat,
+) {
 	ctxCheck := mapp.BaseApp.NewContext(true, abci.Header{})
 	delegation, found := keeper.GetDelegation(ctxCheck, delegatorAddr, validatorAddr)
 	if expFound {
 		require.True(t, found)
-		assert.True(sdk.RatEq(t, expShares, delegation.Shares))
+		require.True(sdk.RatEq(t, expShares, delegation.Shares))
+
 		return
 	}
+
 	require.False(t, found)
 }
 
 func TestStakeMsgs(t *testing.T) {
-	mapp, keeper := getMockApp(t)
+	mApp, keeper := getMockApp(t)
 
-	genCoin := sdk.Coin{"steak", sdk.NewInt(42)}
+	genCoin := sdk.NewCoin("steak", 42)
-	bondCoin := sdk.Coin{"steak", sdk.NewInt(10)}
+	bondCoin := sdk.NewCoin("steak", 10)
 
 	acc1 := &auth.BaseAccount{
 		Address: addr1,
@@ -108,56 +111,51 @@ func TestStakeMsgs(t *testing.T) {
 	}
 	accs := []auth.Account{acc1, acc2}
 
-	mock.SetGenesis(mapp, accs)
+	mock.SetGenesis(mApp, accs)
-	mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin})
+	mock.CheckBalance(t, mApp, addr1, sdk.Coins{genCoin})
-	mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin})
+	mock.CheckBalance(t, mApp, addr2, sdk.Coins{genCoin})
-
-	////////////////////
-	// Create Validator
 
+	// create validator
 	description := NewDescription("foo_moniker", "", "", "")
 	createValidatorMsg := NewMsgCreateValidator(
 		addr1, priv1.PubKey(), bondCoin, description,
 	)
-	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{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, keeper, addr1, true)
+	mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{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, keeper, 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()))
 
 	// check the bond that should have been created as well
-	checkDelegation(t, mapp, keeper, addr1, addr1, true, sdk.NewRat(10))
+	checkDelegation(t, mApp, keeper, addr1, addr1, true, sdk.NewRat(10))
-
-	////////////////////
-	// Edit Validator
 
+	// edit the validator
 	description = NewDescription("bar_moniker", "", "", "")
 	editValidatorMsg := NewMsgEditValidator(addr1, description)
-	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{editValidatorMsg}, []int64{0}, []int64{1}, true, priv1)
+
-	validator = checkValidator(t, mapp, keeper, addr1, true)
+	mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{editValidatorMsg}, []int64{0}, []int64{1}, true, priv1)
+	validator = checkValidator(t, mApp, keeper, addr1, true)
 	require.Equal(t, description, validator.Description)
 
-	////////////////////
+	// delegate
-	// Delegate
+	mock.CheckBalance(t, mApp, addr2, sdk.Coins{genCoin})
-
-	mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin})
 	delegateMsg := NewMsgDelegate(addr2, addr1, bondCoin)
-	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{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))
 
-	////////////////////
+	mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{delegateMsg}, []int64{1}, []int64{0}, true, priv2)
-	// Begin Unbonding
+	mock.CheckBalance(t, mApp, addr2, sdk.Coins{genCoin.Minus(bondCoin)})
+	checkDelegation(t, mApp, keeper, addr2, addr1, true, sdk.NewRat(10))
 
+	// begin unbonding
 	beginUnbondingMsg := NewMsgBeginUnbonding(addr2, addr1, sdk.NewRat(10))
-	mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{beginUnbondingMsg}, []int64{1}, []int64{1}, true, priv2)
+	mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{beginUnbondingMsg}, []int64{1}, []int64{1}, true, priv2)
 
 	// delegation should exist anymore
-	checkDelegation(t, mapp, keeper, addr2, addr1, false, sdk.Rat{})
+	checkDelegation(t, mApp, keeper, addr2, addr1, false, sdk.Rat{})
 
 	// balance should be the same because bonding not yet complete
-	mock.CheckBalance(t, mapp, addr2, sdk.Coins{genCoin.Minus(bondCoin)})
+	mock.CheckBalance(t, mApp, addr2, sdk.Coins{genCoin.Minus(bondCoin)})
 }

x/stake/genesis.go

@@ -1,50 +1,58 @@
 package stake
 
 import (
-	tmtypes "github.com/tendermint/tendermint/types"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/stake/types"
+	tmtypes "github.com/tendermint/tendermint/types"
 )
 
-// InitGenesis - store genesis parameters
+// InitGenesis sets the pool and parameters for the provided keeper and
+// initializes the IntraTxCounter. For each validator in data, it sets that
+// validator in the keeper along with manually setting the indexes. In
+// addition, it also sets any delegations found in data. Finally, it updates
+// the bonded validators.
 func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) {
 	keeper.SetPool(ctx, data.Pool)
 	keeper.SetNewParams(ctx, data.Params)
 	keeper.InitIntraTxCounter(ctx)
-	for _, validator := range data.Validators {
 
-		// set validator
+	for _, validator := range data.Validators {
 		keeper.SetValidator(ctx, validator)
 
-		// manually set indexes for the first time
+		// Manually set indexes for the first time
 		keeper.SetValidatorByPubKeyIndex(ctx, validator)
 		keeper.SetValidatorByPowerIndex(ctx, validator, data.Pool)
+
 		if validator.Status() == sdk.Bonded {
 			keeper.SetValidatorBondedIndex(ctx, validator)
 		}
 	}
+
 	for _, bond := range data.Bonds {
 		keeper.SetDelegation(ctx, bond)
 	}
+
 	keeper.UpdateBondedValidatorsFull(ctx)
 }
 
-// WriteGenesis - output genesis parameters
+// WriteGenesis returns a GenesisState for a given context and keeper. The
+// GenesisState will contain the pool, params, validators, and bonds found in
+// the keeper.
 func WriteGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState {
 	pool := keeper.GetPool(ctx)
 	params := keeper.GetParams(ctx)
 	validators := keeper.GetAllValidators(ctx)
 	bonds := keeper.GetAllDelegations(ctx)
+
 	return types.GenesisState{
-		pool,
+		Pool:       pool,
-		params,
+		Params:     params,
-		validators,
+		Validators: validators,
-		bonds,
+		Bonds:      bonds,
 	}
 }
 
-// WriteValidators - output current validator set
+// WriteValidators returns a slice of bonded genesis validators.
 func WriteValidators(ctx sdk.Context, keeper Keeper) (vals []tmtypes.GenesisValidator) {
 	keeper.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) (stop bool) {
 		vals = append(vals, tmtypes.GenesisValidator{
@@ -52,7 +60,9 @@ func WriteValidators(ctx sdk.Context, keeper Keeper) (vals []tmtypes.GenesisVali
 			Power:  validator.GetPower().RoundInt64(),
 			Name:   validator.GetMoniker(),
 		})
+
 		return false
 	})
+
 	return
 }

x/stake/stake.go

@@ -7,30 +7,29 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/stake/types"
 )
 
-// keeper
+type (
-type Keeper = keeper.Keeper
+	Keeper                = keeper.Keeper
-
+	Validator             = types.Validator
-var NewKeeper = keeper.NewKeeper
+	Description           = types.Description
-
+	Delegation            = types.Delegation
-// types
+	UnbondingDelegation   = types.UnbondingDelegation
-type Validator = types.Validator
+	Redelegation          = types.Redelegation
-type Description = types.Description
+	Params                = types.Params
-type Delegation = types.Delegation
+	Pool                  = types.Pool
-type UnbondingDelegation = types.UnbondingDelegation
+	PoolShares            = types.PoolShares
-type Redelegation = types.Redelegation
+	MsgCreateValidator    = types.MsgCreateValidator
-type Params = types.Params
+	MsgEditValidator      = types.MsgEditValidator
-type Pool = types.Pool
+	MsgDelegate           = types.MsgDelegate
-type PoolShares = types.PoolShares
+	MsgBeginUnbonding     = types.MsgBeginUnbonding
-type MsgCreateValidator = types.MsgCreateValidator
+	MsgCompleteUnbonding  = types.MsgCompleteUnbonding
-type MsgEditValidator = types.MsgEditValidator
+	MsgBeginRedelegate    = types.MsgBeginRedelegate
-type MsgDelegate = types.MsgDelegate
+	MsgCompleteRedelegate = types.MsgCompleteRedelegate
-type MsgBeginUnbonding = types.MsgBeginUnbonding
+	GenesisState          = types.GenesisState
-type MsgCompleteUnbonding = types.MsgCompleteUnbonding
+)
-type MsgBeginRedelegate = types.MsgBeginRedelegate
-type MsgCompleteRedelegate = types.MsgCompleteRedelegate
-type GenesisState = types.GenesisState
 
 var (
+	NewKeeper = keeper.NewKeeper
+
 	GetValidatorKey              = keeper.GetValidatorKey
 	GetValidatorByPubKeyIndexKey = keeper.GetValidatorByPubKeyIndexKey
 	GetValidatorsBondedIndexKey  = keeper.GetValidatorsBondedIndexKey
@@ -72,7 +71,6 @@ var (
 	DefaultGenesisState = types.DefaultGenesisState
 	RegisterWire        = types.RegisterWire
 
-	// messages
 	NewMsgCreateValidator    = types.NewMsgCreateValidator
 	NewMsgEditValidator      = types.NewMsgEditValidator
 	NewMsgDelegate           = types.NewMsgDelegate
@@ -82,7 +80,6 @@ var (
 	NewMsgCompleteRedelegate = types.NewMsgCompleteRedelegate
 )
 
-// errors
 const (
 	DefaultCodespace      = types.DefaultCodespace
 	CodeInvalidValidator  = types.CodeInvalidValidator
@@ -126,7 +123,6 @@ var (
 	ErrMissingSignature      = types.ErrMissingSignature
 )
 
-// tags
 var (
 	ActionCreateValidator      = tags.ActionCreateValidator
 	ActionEditValidator        = tags.ActionEditValidator

x/stake/types/delegation.go

@@ -17,7 +17,7 @@ type Delegation struct {
 	Height        int64       `json:"height"` // Last height bond updated
 }
 
-// two are equal
+// Equal returns a boolean determining if two Delegation types are identical.
 func (d Delegation) Equal(d2 Delegation) bool {
 	return bytes.Equal(d.DelegatorAddr, d2.DelegatorAddr) &&
 		bytes.Equal(d.ValidatorAddr, d2.ValidatorAddr) &&
@@ -33,16 +33,20 @@ func (d Delegation) GetDelegator() sdk.Address { return d.DelegatorAddr }
 func (d Delegation) GetValidator() sdk.Address { return d.ValidatorAddr }
 func (d Delegation) GetBondShares() sdk.Rat    { return d.Shares }
 
-//Human Friendly pretty printer
+// HumanReadableString returns a human readable string representation of a
+// Delegation. An error is returned if the Delegation's delegator or validator
+// addresses cannot be Bech32 encoded.
 func (d Delegation) HumanReadableString() (string, error) {
 	bechAcc, err := sdk.Bech32ifyAcc(d.DelegatorAddr)
 	if err != nil {
 		return "", err
 	}
+
 	bechVal, err := sdk.Bech32ifyAcc(d.ValidatorAddr)
 	if err != nil {
 		return "", err
 	}
+
 	resp := "Delegation \n"
 	resp += fmt.Sprintf("Delegator: %s\n", bechAcc)
 	resp += fmt.Sprintf("Validator: %s\n", bechVal)
@@ -50,12 +54,9 @@ func (d Delegation) HumanReadableString() (string, error) {
 	resp += fmt.Sprintf("Height: %d", d.Height)
 
 	return resp, nil
-
 }
 
-//__________________________________________________________________
+// UnbondingDelegation reflects a delegation's passive unbonding queue.
-
-// element stored to represent the passive unbonding queue
 type UnbondingDelegation struct {
 	DelegatorAddr  sdk.Address `json:"delegator_addr"`  // delegator
 	ValidatorAddr  sdk.Address `json:"validator_addr"`  // validator unbonding from owner addr
@@ -65,23 +66,28 @@ type UnbondingDelegation struct {
 	Balance        sdk.Coin    `json:"balance"`         // atoms to receive at completion
 }
 
-// nolint
+// Equal returns a boolean determining if two UnbondingDelegation types are
+// identical.
 func (d UnbondingDelegation) Equal(d2 UnbondingDelegation) bool {
 	bz1 := MsgCdc.MustMarshalBinary(&d)
 	bz2 := MsgCdc.MustMarshalBinary(&d2)
 	return bytes.Equal(bz1, bz2)
 }
 
-//Human Friendly pretty printer
+// HumanReadableString returns a human readable string representation of an
+// UnbondingDelegation. An error is returned if the UnbondingDelegation's
+// delegator or validator addresses cannot be Bech32 encoded.
 func (d UnbondingDelegation) HumanReadableString() (string, error) {
 	bechAcc, err := sdk.Bech32ifyAcc(d.DelegatorAddr)
 	if err != nil {
 		return "", err
 	}
+
 	bechVal, err := sdk.Bech32ifyAcc(d.ValidatorAddr)
 	if err != nil {
 		return "", err
 	}
+
 	resp := "Unbonding Delegation \n"
 	resp += fmt.Sprintf("Delegator: %s\n", bechAcc)
 	resp += fmt.Sprintf("Validator: %s\n", bechVal)
@@ -93,9 +99,7 @@ func (d UnbondingDelegation) HumanReadableString() (string, error) {
 
 }
 
-//__________________________________________________________________
+// Redelegation reflects a delegation's passive re-delegation queue.
-
-// element stored to represent the passive redelegation queue
 type Redelegation struct {
 	DelegatorAddr    sdk.Address `json:"delegator_addr"`     // delegator
 	ValidatorSrcAddr sdk.Address `json:"validator_src_addr"` // validator redelegation source owner addr
@@ -108,27 +112,32 @@ type Redelegation struct {
 	SharesDst        sdk.Rat     `json:"shares_dst"`         // amount of destination shares redelegating
 }
 
-// nolint
+// Equal returns a boolean determining if two Redelegation types are identical.
 func (d Redelegation) Equal(d2 Redelegation) bool {
 	bz1 := MsgCdc.MustMarshalBinary(&d)
 	bz2 := MsgCdc.MustMarshalBinary(&d2)
 	return bytes.Equal(bz1, bz2)
 }
 
-//Human Friendly pretty printer
+// HumanReadableString returns a human readable string representation of a
+// Redelegation. An error is returned if the UnbondingDelegation's delegator or
+// validator addresses cannot be Bech32 encoded.
 func (d Redelegation) HumanReadableString() (string, error) {
 	bechAcc, err := sdk.Bech32ifyAcc(d.DelegatorAddr)
 	if err != nil {
 		return "", err
 	}
+
 	bechValSrc, err := sdk.Bech32ifyAcc(d.ValidatorSrcAddr)
 	if err != nil {
 		return "", err
 	}
+
 	bechValDst, err := sdk.Bech32ifyAcc(d.ValidatorDstAddr)
 	if err != nil {
 		return "", err
 	}
+
 	resp := "Redelegation \n"
 	resp += fmt.Sprintf("Delegator: %s\n", bechAcc)
 	resp += fmt.Sprintf("Source Validator: %s\n", bechValSrc)

x/stake/types/delegation_test.go

@@ -0,0 +1,116 @@
+package types
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDelegationEqual(t *testing.T) {
+	d1 := Delegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+		Shares:        sdk.NewRat(100),
+	}
+	d2 := Delegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+		Shares:        sdk.NewRat(100),
+	}
+
+	ok := d1.Equal(d2)
+	require.True(t, ok)
+
+	d2.ValidatorAddr = addr3
+	d2.Shares = sdk.NewRat(200)
+
+	ok = d1.Equal(d2)
+	require.False(t, ok)
+}
+
+func TestDelegationHumanReadableString(t *testing.T) {
+	d := Delegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+		Shares:        sdk.NewRat(100),
+	}
+
+	// NOTE: Being that the validator's keypair is random, we cannot test the
+	// actual contents of the string.
+	valStr, err := d.HumanReadableString()
+	require.Nil(t, err)
+	require.NotEmpty(t, valStr)
+}
+
+func TestUnbondingDelegationEqual(t *testing.T) {
+	ud1 := UnbondingDelegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+	}
+	ud2 := UnbondingDelegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+	}
+
+	ok := ud1.Equal(ud2)
+	require.True(t, ok)
+
+	ud2.ValidatorAddr = addr3
+	ud2.MinTime = 20 * 20 * 2
+
+	ok = ud1.Equal(ud2)
+	require.False(t, ok)
+}
+
+func TestUnbondingDelegationHumanReadableString(t *testing.T) {
+	ud := UnbondingDelegation{
+		DelegatorAddr: addr1,
+		ValidatorAddr: addr2,
+	}
+
+	// NOTE: Being that the validator's keypair is random, we cannot test the
+	// actual contents of the string.
+	valStr, err := ud.HumanReadableString()
+	require.Nil(t, err)
+	require.NotEmpty(t, valStr)
+}
+
+func TestRedelegationEqual(t *testing.T) {
+	r1 := Redelegation{
+		DelegatorAddr:    addr1,
+		ValidatorSrcAddr: addr2,
+		ValidatorDstAddr: addr3,
+	}
+	r2 := Redelegation{
+		DelegatorAddr:    addr1,
+		ValidatorSrcAddr: addr2,
+		ValidatorDstAddr: addr3,
+	}
+
+	ok := r1.Equal(r2)
+	require.True(t, ok)
+
+	r2.SharesDst = sdk.NewRat(10)
+	r2.SharesSrc = sdk.NewRat(20)
+	r2.MinTime = 20 * 20 * 2
+
+	ok = r1.Equal(r2)
+	require.False(t, ok)
+}
+
+func TestRedelegationHumanReadableString(t *testing.T) {
+	r := Redelegation{
+		DelegatorAddr:    addr1,
+		ValidatorSrcAddr: addr2,
+		ValidatorDstAddr: addr3,
+		SharesDst:        sdk.NewRat(10),
+		SharesSrc:        sdk.NewRat(20),
+	}
+
+	// NOTE: Being that the validator's keypair is random, we cannot test the
+	// actual contents of the string.
+	valStr, err := r.HumanReadableString()
+	require.Nil(t, err)
+	require.NotEmpty(t, valStr)
+}

x/stake/types/errors.go

@@ -25,97 +25,118 @@ const (
 func ErrNilValidatorAddr(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidInput, "validator address is nil")
 }
+
 func ErrNoValidatorFound(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "validator does not exist for that address")
 }
+
 func ErrValidatorAlreadyExists(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "validator already exist, cannot re-create validator")
 }
+
 func ErrValidatorRevoked(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "validator for this address is currently revoked")
 }
+
 func ErrBadRemoveValidator(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "error removing validator")
 }
+
 func ErrDescriptionLength(codespace sdk.CodespaceType, descriptor string, got, max int) sdk.Error {
 	msg := fmt.Sprintf("bad description length for %v, got length %v, max is %v", descriptor, got, max)
 	return sdk.NewError(codespace, CodeInvalidValidator, msg)
 }
+
 func ErrCommissionNegative(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "commission must be positive")
 }
+
 func ErrCommissionHuge(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be more than 100%")
 }
 
-// delegation
 func ErrNilDelegatorAddr(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidInput, "delegator address is nil")
 }
+
 func ErrBadDenom(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "invalid coin denomination")
 }
+
 func ErrBadDelegationAmount(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "amount must be > 0")
 }
+
 func ErrNoDelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "no delegation for this (address, validator) pair")
 }
+
 func ErrBadDelegatorAddr(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "delegator does not exist for that address")
 }
+
 func ErrNoDelegatorForAddress(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "delegator does not contain this delegation")
 }
+
 func ErrInsufficientShares(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "insufficient delegation shares")
 }
+
 func ErrDelegationValidatorEmpty(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "cannot delegate to an empty validator")
 }
+
 func ErrNotEnoughDelegationShares(codespace sdk.CodespaceType, shares string) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, fmt.Sprintf("not enough shares only have %v", shares))
 }
+
 func ErrBadSharesAmount(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "shares must be > 0")
 }
+
 func ErrBadSharesPrecision(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation,
 		fmt.Sprintf("shares denominator must be < %s, try reducing the number of decimal points",
 			maximumBondingRationalDenominator.String()),
 	)
 }
+
 func ErrBadSharesPercent(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "shares percent must be >0 and <=1")
 }
 
-// redelegation
 func ErrNotMature(codespace sdk.CodespaceType, operation, descriptor string, got, min int64) sdk.Error {
 	msg := fmt.Sprintf("%v is not mature requires a min %v of %v, currently it is %v",
 		operation, descriptor, got, min)
 	return sdk.NewError(codespace, CodeUnauthorized, msg)
 }
+
 func ErrNoUnbondingDelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "no unbonding delegation found")
 }
+
 func ErrNoRedelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "no redelegation found")
 }
+
 func ErrBadRedelegationDst(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation, "redelegation validator not found")
 }
+
 func ErrTransitiveRedelegation(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDelegation,
 		"redelegation to this validator already in progress, first redelegation to this validator must complete before next redelegation")
 }
 
-// messages
 func ErrBothShareMsgsGiven(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidInput, "both shares amount and shares percent provided")
 }
+
 func ErrNeitherShareMsgsGiven(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidInput, "neither shares amount nor shares percent provided")
 }
+
 func ErrMissingSignature(codespace sdk.CodespaceType) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidValidator, "missing signature")
 }

x/stake/types/msg_test.go

@@ -10,9 +10,9 @@ import (
 )
 
 var (
-	coinPos  = sdk.Coin{"steak", sdk.NewInt(1000)}
+	coinPos  = sdk.NewCoin("steak", 1000)
-	coinZero = sdk.Coin{"steak", sdk.NewInt(0)}
+	coinZero = sdk.NewCoin("steak", 0)
-	coinNeg  = sdk.Coin{"steak", sdk.NewInt(-10000)}
+	coinNeg  = sdk.NewCoin("steak", -10000)
 )
 
 // test ValidateBasic for MsgCreateValidator
@@ -197,29 +197,3 @@ func TestMsgCompleteUnbonding(t *testing.T) {
 		}
 	}
 }
-
-// TODO introduce with go-amino
-//func TestSerializeMsg(t *testing.T) {
-
-//// make sure all types construct properly
-//bondAmt := 1234321
-//bond := sdk.Coin{Denom: "atom", Amount: int64(bondAmt)}
-
-//tests := []struct {
-//tx sdk.Msg
-//}{
-//{NewMsgCreateValidator(addr1, pk1, bond, Description{})},
-//{NewMsgEditValidator(addr1, Description{})},
-//{NewMsgDelegate(addr1, addr2, bond)},
-//{NewMsgUnbond(addr1, addr2, strconv.Itoa(bondAmt))},
-//}
-
-//for i, tc := range tests {
-//var tx sdk.Tx
-//bs := wire.BinaryBytes(tc.tx)
-//err := wire.ReadBinaryBytes(bs, &tx)
-//if require.NoError(t, err, "%d", i) {
-//require.Equal(t, tc.tx, tx, "%d", i)
-//}
-//}
-//}

x/stake/types/params.go

@@ -6,6 +6,10 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+// defaultUnbondingTime reflects three weeks in seconds as the default
+// unbonding time.
+const defaultUnbondingTime int64 = 60 * 60 * 24 * 3
+
 // Params defines the high level settings for staking
 type Params struct {
 	InflationRateChange sdk.Rat `json:"inflation_rate_change"` // maximum annual change in inflation rate
@@ -19,21 +23,21 @@ type Params struct {
 	BondDenom     string `json:"bond_denom"`     // bondable coin denomination
 }
 
-// nolint
+// Equal returns a boolean determining if two Param types are identical.
 func (p Params) Equal(p2 Params) bool {
 	bz1 := MsgCdc.MustMarshalBinary(&p)
 	bz2 := MsgCdc.MustMarshalBinary(&p2)
 	return bytes.Equal(bz1, bz2)
 }
 
-// default params
+// DefaultParams returns a default set of parameters.
 func DefaultParams() Params {
 	return Params{
 		InflationRateChange: sdk.NewRat(13, 100),
 		InflationMax:        sdk.NewRat(20, 100),
 		InflationMin:        sdk.NewRat(7, 100),
 		GoalBonded:          sdk.NewRat(67, 100),
-		UnbondingTime:       60 * 60 * 24 * 3, // 3 weeks in seconds
+		UnbondingTime:       defaultUnbondingTime,
 		MaxValidators:       100,
 		BondDenom:           "steak",
 	}

x/stake/types/params_test.go

@@ -0,0 +1,21 @@
+package types
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParamsEqual(t *testing.T) {
+	p1 := DefaultParams()
+	p2 := DefaultParams()
+
+	ok := p1.Equal(p2)
+	require.True(t, ok)
+
+	p2.UnbondingTime = 60 * 60 * 24 * 2
+	p2.BondDenom = "soup"
+
+	ok = p1.Equal(p2)
+	require.False(t, ok)
+}

x/stake/types/pool_test.go

@@ -3,11 +3,24 @@ package types
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
 )
 
+func TestPoolEqual(t *testing.T) {
+	p1 := InitialPool()
+	p2 := InitialPool()
+
+	ok := p1.Equal(p2)
+	require.True(t, ok)
+
+	p2.BondedTokens = 3
+	p2.BondedShares = sdk.NewRat(10)
+
+	ok = p1.Equal(p2)
+	require.False(t, ok)
+}
+
 func TestBondedRatio(t *testing.T) {
 	pool := InitialPool()
 	pool.LooseTokens = 1
@@ -62,10 +75,10 @@ func TestUnbondedShareExRate(t *testing.T) {
 }
 
 func TestAddTokensBonded(t *testing.T) {
-
 	poolA := InitialPool()
 	poolA.LooseTokens = 10
 	require.Equal(t, poolA.BondedShareExRate(), sdk.OneRat())
+
 	poolB, sharesB := poolA.addTokensBonded(10)
 	require.Equal(t, poolB.BondedShareExRate(), sdk.OneRat())
 
@@ -78,10 +91,10 @@ func TestAddTokensBonded(t *testing.T) {
 }
 
 func TestRemoveSharesBonded(t *testing.T) {
-
 	poolA := InitialPool()
 	poolA.LooseTokens = 10
 	require.Equal(t, poolA.BondedShareExRate(), sdk.OneRat())
+
 	poolB, tokensB := poolA.removeSharesBonded(sdk.NewRat(10))
 	require.Equal(t, poolB.BondedShareExRate(), sdk.OneRat())
 
@@ -94,10 +107,10 @@ func TestRemoveSharesBonded(t *testing.T) {
 }
 
 func TestAddTokensUnbonded(t *testing.T) {
-
 	poolA := InitialPool()
 	poolA.LooseTokens = 10
 	require.Equal(t, poolA.UnbondedShareExRate(), sdk.OneRat())
+
 	poolB, sharesB := poolA.addTokensUnbonded(10)
 	require.Equal(t, poolB.UnbondedShareExRate(), sdk.OneRat())
 
@@ -110,11 +123,11 @@ func TestAddTokensUnbonded(t *testing.T) {
 }
 
 func TestRemoveSharesUnbonded(t *testing.T) {
-
 	poolA := InitialPool()
 	poolA.UnbondedTokens = 10
 	poolA.UnbondedShares = sdk.NewRat(10)
 	require.Equal(t, poolA.UnbondedShareExRate(), sdk.OneRat())
+
 	poolB, tokensB := poolA.removeSharesUnbonded(sdk.NewRat(10))
 	require.Equal(t, poolB.UnbondedShareExRate(), sdk.OneRat())
 

x/stake/types/shares.go

@@ -4,18 +4,19 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-// pool shares held by a validator
+// PoolShares reflects the shares of a validator in a pool.
 type PoolShares struct {
 	Status sdk.BondStatus `json:"status"`
-	Amount sdk.Rat        `json:"amount"` // total shares of type ShareKind
+	Amount sdk.Rat        `json:"amount"`
 }
 
-// only the vitals - does not check bond height of IntraTxCounter
+// Equal returns a boolean determining of two PoolShares are identical.
 func (s PoolShares) Equal(s2 PoolShares) bool {
 	return s.Status == s2.Status &&
 		s.Amount.Equal(s2.Amount)
 }
 
+// NewUnbondedShares returns a new PoolShares with a specified unbonded amount.
 func NewUnbondedShares(amount sdk.Rat) PoolShares {
 	return PoolShares{
 		Status: sdk.Unbonded,
@@ -23,6 +24,8 @@ func NewUnbondedShares(amount sdk.Rat) PoolShares {
 	}
 }
 
+// NewUnbondingShares returns a new PoolShares with a specified unbonding
+// amount.
 func NewUnbondingShares(amount sdk.Rat) PoolShares {
 	return PoolShares{
 		Status: sdk.Unbonding,
@@ -30,6 +33,7 @@ func NewUnbondingShares(amount sdk.Rat) PoolShares {
 	}
 }
 
+// NewBondedShares returns a new PoolSahres with a specified bonding amount.
 func NewBondedShares(amount sdk.Rat) PoolShares {
 	return PoolShares{
 		Status: sdk.Bonded,
@@ -37,9 +41,7 @@ func NewBondedShares(amount sdk.Rat) PoolShares {
 	}
 }
 
-//_________________________________________________________________________________________________________
+// Unbonded returns the amount of unbonded shares.
-
-// amount of unbonded shares
 func (s PoolShares) Unbonded() sdk.Rat {
 	if s.Status == sdk.Unbonded {
 		return s.Amount
@@ -47,7 +49,7 @@ func (s PoolShares) Unbonded() sdk.Rat {
 	return sdk.ZeroRat()
 }
 
-// amount of unbonding shares
+// Unbonding returns the amount of unbonding shares.
 func (s PoolShares) Unbonding() sdk.Rat {
 	if s.Status == sdk.Unbonding {
 		return s.Amount
@@ -55,7 +57,7 @@ func (s PoolShares) Unbonding() sdk.Rat {
 	return sdk.ZeroRat()
 }
 
-// amount of bonded shares
+// Bonded returns amount of bonded shares.
 func (s PoolShares) Bonded() sdk.Rat {
 	if s.Status == sdk.Bonded {
 		return s.Amount
@@ -63,64 +65,80 @@ func (s PoolShares) Bonded() sdk.Rat {
 	return sdk.ZeroRat()
 }
 
-//_________________________________________________________________________________________________________
+// ToUnbonded returns the equivalent amount of pool shares if the shares were
-
+// unbonded.
-// equivalent amount of shares if the shares were unbonded
 func (s PoolShares) ToUnbonded(p Pool) PoolShares {
 	var amount sdk.Rat
+
 	switch s.Status {
 	case sdk.Bonded:
-		exRate := p.BondedShareExRate().Quo(p.UnbondedShareExRate()) // (tok/bondedshr)/(tok/unbondedshr) = unbondedshr/bondedshr
+		// (tok/bondedshr)/(tok/unbondedshr) = unbondedshr/bondedshr
-		amount = s.Amount.Mul(exRate)                                // bondedshr*unbondedshr/bondedshr = unbondedshr
+		exRate := p.BondedShareExRate().Quo(p.UnbondedShareExRate())
+		// bondedshr*unbondedshr/bondedshr = unbondedshr
+		amount = s.Amount.Mul(exRate)
 	case sdk.Unbonding:
-		exRate := p.UnbondingShareExRate().Quo(p.UnbondedShareExRate()) // (tok/unbondingshr)/(tok/unbondedshr) = unbondedshr/unbondingshr
+		// (tok/unbondingshr)/(tok/unbondedshr) = unbondedshr/unbondingshr
-		amount = s.Amount.Mul(exRate)                                   // unbondingshr*unbondedshr/unbondingshr = unbondedshr
+		exRate := p.UnbondingShareExRate().Quo(p.UnbondedShareExRate())
+		// unbondingshr*unbondedshr/unbondingshr = unbondedshr
+		amount = s.Amount.Mul(exRate)
 	case sdk.Unbonded:
 		amount = s.Amount
 	}
+
 	return NewUnbondedShares(amount)
 }
 
-// equivalent amount of shares if the shares were unbonding
+// ToUnbonding returns the equivalent amount of pool shares if the shares were
+// unbonding.
 func (s PoolShares) ToUnbonding(p Pool) PoolShares {
 	var amount sdk.Rat
+
 	switch s.Status {
 	case sdk.Bonded:
-		exRate := p.BondedShareExRate().Quo(p.UnbondingShareExRate()) // (tok/bondedshr)/(tok/unbondingshr) = unbondingshr/bondedshr
+		// (tok/bondedshr)/(tok/unbondingshr) = unbondingshr/bondedshr
-		amount = s.Amount.Mul(exRate)                                 // bondedshr*unbondingshr/bondedshr = unbondingshr
+		exRate := p.BondedShareExRate().Quo(p.UnbondingShareExRate())
+		// bondedshr*unbondingshr/bondedshr = unbondingshr
+		amount = s.Amount.Mul(exRate)
 	case sdk.Unbonding:
 		amount = s.Amount
 	case sdk.Unbonded:
-		exRate := p.UnbondedShareExRate().Quo(p.UnbondingShareExRate()) // (tok/unbondedshr)/(tok/unbondingshr) = unbondingshr/unbondedshr
+		// (tok/unbondedshr)/(tok/unbondingshr) = unbondingshr/unbondedshr
-		amount = s.Amount.Mul(exRate)                                   // unbondedshr*unbondingshr/unbondedshr = unbondingshr
+		exRate := p.UnbondedShareExRate().Quo(p.UnbondingShareExRate())
+		// unbondedshr*unbondingshr/unbondedshr = unbondingshr
+		amount = s.Amount.Mul(exRate)
 	}
+
 	return NewUnbondingShares(amount)
 }
 
-// equivalent amount of shares if the shares were bonded
+// ToBonded the equivalent amount of pool shares if the shares were bonded.
 func (s PoolShares) ToBonded(p Pool) PoolShares {
 	var amount sdk.Rat
+
 	switch s.Status {
 	case sdk.Bonded:
 		amount = s.Amount
 	case sdk.Unbonding:
-		exRate := p.UnbondingShareExRate().Quo(p.BondedShareExRate()) // (tok/ubshr)/(tok/bshr) = bshr/ubshr
+		// (tok/ubshr)/(tok/bshr) = bshr/ubshr
-		amount = s.Amount.Mul(exRate)                                 // ubshr*bshr/ubshr = bshr
+		exRate := p.UnbondingShareExRate().Quo(p.BondedShareExRate())
+		// ubshr*bshr/ubshr = bshr
+		amount = s.Amount.Mul(exRate)
 	case sdk.Unbonded:
-		exRate := p.UnbondedShareExRate().Quo(p.BondedShareExRate()) // (tok/ubshr)/(tok/bshr) = bshr/ubshr
+		// (tok/ubshr)/(tok/bshr) = bshr/ubshr
-		amount = s.Amount.Mul(exRate)                                // ubshr*bshr/ubshr = bshr
+		exRate := p.UnbondedShareExRate().Quo(p.BondedShareExRate())
+		// ubshr*bshr/ubshr = bshr
+		amount = s.Amount.Mul(exRate)
 	}
+
 	return NewUnbondedShares(amount)
 }
 
-//_________________________________________________________________________________________________________
+// Tokens returns the equivalent amount of tokens contained by the pool shares
-
+// for a given pool.
-// TODO better tests
-// get the equivalent amount of tokens contained by the shares
 func (s PoolShares) Tokens(p Pool) sdk.Rat {
 	switch s.Status {
 	case sdk.Bonded:
-		return p.BondedShareExRate().Mul(s.Amount) // (tokens/shares) * shares
+		return p.BondedShareExRate().Mul(s.Amount)
 	case sdk.Unbonding:
 		return p.UnbondingShareExRate().Mul(s.Amount)
 	case sdk.Unbonded:

x/stake/types/shares_test.go

@@ -0,0 +1,35 @@
+package types
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPoolSharesTokens(t *testing.T) {
+	pool := InitialPool()
+	pool.LooseTokens = 10
+
+	val := Validator{
+		Owner:           addr1,
+		PubKey:          pk1,
+		PoolShares:      NewBondedShares(sdk.NewRat(100)),
+		DelegatorShares: sdk.NewRat(100),
+	}
+
+	pool.BondedTokens = val.PoolShares.Bonded().RoundInt64()
+	pool.BondedShares = val.PoolShares.Bonded()
+
+	poolShares := NewBondedShares(sdk.NewRat(50))
+	tokens := poolShares.Tokens(pool)
+	require.Equal(t, int64(50), tokens.RoundInt64())
+
+	poolShares = NewUnbondingShares(sdk.NewRat(50))
+	tokens = poolShares.Tokens(pool)
+	require.Equal(t, int64(50), tokens.RoundInt64())
+
+	poolShares = NewUnbondedShares(sdk.NewRat(50))
+	tokens = poolShares.Tokens(pool)
+	require.Equal(t, int64(50), tokens.RoundInt64())
+}

x/stake/types/test_utils.go

@@ -11,7 +11,6 @@ import (
 )
 
 var (
-	// dummy pubkeys/addresses
 	pk1   = crypto.GenPrivKeyEd25519().PubKey()
 	pk2   = crypto.GenPrivKeyEd25519().PubKey()
 	pk3   = crypto.GenPrivKeyEd25519().PubKey()
@@ -23,18 +22,20 @@ var (
 	emptyPubkey crypto.PubKey
 )
 
-//______________________________________________________________
+// Operation reflects any operation that transforms staking state. It takes in
-
+// a RNG instance, pool, validator and returns an updated pool, updated
-// any operation that transforms staking state
+// validator, delta tokens, and descriptive message.
-// takes in RNG instance, pool, validator
-// returns updated pool, updated validator, delta tokens, descriptive message
 type Operation func(r *rand.Rand, pool Pool, c Validator) (Pool, Validator, int64, string)
 
-// operation: bond or unbond a validator depending on current status
+// OpBondOrUnbond implements an operation that bonds or unbonds a validator
+// depending on current status.
 // nolint: unparam
 func OpBondOrUnbond(r *rand.Rand, pool Pool, val Validator) (Pool, Validator, int64, string) {
-	var msg string
+	var (
-	var newStatus sdk.BondStatus
+		msg       string
+		newStatus sdk.BondStatus
+	)
+
 	if val.Status() == sdk.Bonded {
 		msg = fmt.Sprintf("sdk.Unbonded previously bonded validator %s (poolShares: %v, delShares: %v, DelegatorShareExRate: %v)",
 			val.Owner, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool))
@@ -45,21 +46,27 @@ func OpBondOrUnbond(r *rand.Rand, pool Pool, val Validator) (Pool, Validator, in
 			val.Owner, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool))
 		newStatus = sdk.Bonded
 	}
+
 	val, pool = val.UpdateStatus(pool, newStatus)
 	return pool, val, 0, msg
 }
 
-// operation: add a random number of tokens to a validator
+// OpAddTokens implements an operation that adds a random number of tokens to a
+// validator.
 func OpAddTokens(r *rand.Rand, pool Pool, val Validator) (Pool, Validator, int64, string) {
-	tokens := int64(r.Int31n(1000))
 	msg := fmt.Sprintf("validator %s (status: %d, poolShares: %v, delShares: %v, DelegatorShareExRate: %v)",
 		val.Owner, val.Status(), val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool))
+
+	tokens := int64(r.Int31n(1000))
 	val, pool, _ = val.AddTokensFromDel(pool, tokens)
 	msg = fmt.Sprintf("Added %d tokens to %s", tokens, msg)
-	return pool, val, -1 * tokens, msg // tokens are removed so for accounting must be negative
+
+	// Tokens are removed so for accounting must be negative
+	return pool, val, -1 * tokens, msg
 }
 
-// operation: remove a random number of shares from a validator
+// OpRemoveShares implements an operation that removes a random number of
+// shares from a validator.
 func OpRemoveShares(r *rand.Rand, pool Pool, val Validator) (Pool, Validator, int64, string) {
 	var shares sdk.Rat
 	for {
@@ -76,7 +83,7 @@ func OpRemoveShares(r *rand.Rand, pool Pool, val Validator) (Pool, Validator, in
 	return pool, val, tokens, msg
 }
 
-// pick a random staking operation
+// RandomOperation returns a random staking operation.
 func RandomOperation(r *rand.Rand) Operation {
 	operations := []Operation{
 		OpBondOrUnbond,
@@ -86,10 +93,11 @@ func RandomOperation(r *rand.Rand) Operation {
 	r.Shuffle(len(operations), func(i, j int) {
 		operations[i], operations[j] = operations[j], operations[i]
 	})
+
 	return operations[0]
 }
 
-// ensure invariants that should always be true are true
+// AssertInvariants ensures invariants that should always be true are true.
 // nolint: unparam
 func AssertInvariants(t *testing.T, msg string,
 	pOrig Pool, cOrig []Validator, pMod Pool, vMods []Validator, tokens int64) {
@@ -105,29 +113,28 @@ func AssertInvariants(t *testing.T, msg string,
 		pOrig.UnbondedTokens, pOrig.BondedTokens,
 		pMod.UnbondedTokens, pMod.BondedTokens, tokens)
 
-	// nonnegative bonded shares
+	// Nonnegative bonded shares
 	require.False(t, pMod.BondedShares.LT(sdk.ZeroRat()),
 		"Negative bonded shares - msg: %v\npOrig: %v\npMod: %v\ntokens: %v\n",
 		msg, pOrig, pMod, tokens)
 
-	// nonnegative unbonded shares
+	// Nonnegative unbonded shares
 	require.False(t, pMod.UnbondedShares.LT(sdk.ZeroRat()),
 		"Negative unbonded shares - msg: %v\npOrig: %v\npMod: %v\ntokens: %v\n",
 		msg, pOrig, pMod, tokens)
 
-	// nonnegative bonded ex rate
+	// Nonnegative bonded ex rate
 	require.False(t, pMod.BondedShareExRate().LT(sdk.ZeroRat()),
 		"Applying operation \"%s\" resulted in negative BondedShareExRate: %d",
 		msg, pMod.BondedShareExRate().RoundInt64())
 
-	// nonnegative unbonded ex rate
+	// Nonnegative unbonded ex rate
 	require.False(t, pMod.UnbondedShareExRate().LT(sdk.ZeroRat()),
 		"Applying operation \"%s\" resulted in negative UnbondedShareExRate: %d",
 		msg, pMod.UnbondedShareExRate().RoundInt64())
 
 	for _, vMod := range vMods {
-
+		// Nonnegative ex rate
-		// nonnegative ex rate
 		require.False(t, vMod.DelegatorShareExRate(pMod).LT(sdk.ZeroRat()),
 			"Applying operation \"%s\" resulted in negative validator.DelegatorShareExRate(): %v (validator.Owner: %s)",
 			msg,
@@ -135,7 +142,7 @@ func AssertInvariants(t *testing.T, msg string,
 			vMod.Owner,
 		)
 
-		// nonnegative poolShares
+		// Nonnegative poolShares
 		require.False(t, vMod.PoolShares.Bonded().LT(sdk.ZeroRat()),
 			"Applying operation \"%s\" resulted in negative validator.PoolShares.Bonded(): %v (validator.DelegatorShares: %v, validator.DelegatorShareExRate: %v, validator.Owner: %s)",
 			msg,
@@ -145,7 +152,7 @@ func AssertInvariants(t *testing.T, msg string,
 			vMod.Owner,
 		)
 
-		// nonnegative delShares
+		// Nonnegative delShares
 		require.False(t, vMod.DelegatorShares.LT(sdk.ZeroRat()),
 			"Applying operation \"%s\" resulted in negative validator.DelegatorShares: %v (validator.PoolShares.Bonded(): %v, validator.DelegatorShareExRate: %v, validator.Owner: %s)",
 			msg,
@@ -154,27 +161,25 @@ func AssertInvariants(t *testing.T, msg string,
 			vMod.DelegatorShareExRate(pMod),
 			vMod.Owner,
 		)
-
 	}
-
 }
 
-//________________________________________________________________________________
+// TODO: refactor this random setup
-// TODO refactor this random setup
 
-// generate a random validator
+// randomValidator generates a random validator.
 // nolint: unparam
 func randomValidator(r *rand.Rand, i int) Validator {
-
 	poolSharesAmt := sdk.NewRat(int64(r.Int31n(10000)))
 	delShares := sdk.NewRat(int64(r.Int31n(10000)))
 
 	var pShares PoolShares
+
 	if r.Float64() < float64(0.5) {
 		pShares = NewBondedShares(poolSharesAmt)
 	} else {
 		pShares = NewUnbondedShares(poolSharesAmt)
 	}
+
 	return Validator{
 		Owner:           addr1,
 		PubKey:          pk1,
@@ -183,7 +188,7 @@ func randomValidator(r *rand.Rand, i int) Validator {
 	}
 }
 
-// generate a random staking state
+// RandomSetup generates a random staking state.
 func RandomSetup(r *rand.Rand, numValidators int) (Pool, []Validator) {
 	pool := InitialPool()
 	pool.LooseTokens = 100000
@@ -191,6 +196,7 @@ func RandomSetup(r *rand.Rand, numValidators int) (Pool, []Validator) {
 	validators := make([]Validator, numValidators)
 	for i := 0; i < numValidators; i++ {
 		validator := randomValidator(r, i)
+
 		if validator.Status() == sdk.Bonded {
 			pool.BondedShares = pool.BondedShares.Add(validator.PoolShares.Bonded())
 			pool.BondedTokens += validator.PoolShares.Bonded().RoundInt64()
@@ -198,7 +204,9 @@ func RandomSetup(r *rand.Rand, numValidators int) (Pool, []Validator) {
 			pool.UnbondedShares = pool.UnbondedShares.Add(validator.PoolShares.Unbonded())
 			pool.UnbondedTokens += validator.PoolShares.Unbonded().RoundInt64()
 		}
+
 		validators[i] = validator
 	}
+
 	return pool, validators
 }

x/stake/types/validator.go

@@ -4,13 +4,14 @@ import (
 	"bytes"
 	"fmt"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto"
 	tmtypes "github.com/tendermint/tendermint/types"
-
-	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+const doNotModifyDescVal = "[do-not-modify]"
+
 // Validator defines the total amount of bond shares and their exchange rate to
 // coins. Accumulation of interest is modelled as an in increase in the
 // exchange rate, and slashing as a decrease.  When coins are delegated to this
@@ -60,15 +61,13 @@ func NewValidator(owner sdk.Address, pubKey crypto.PubKey, description Descripti
 	}
 }
 
-// only the vitals - does not check bond height of IntraTxCounter
+// Equal returns a boolean reflecting if two given validators are identical.
 func (v Validator) Equal(c2 Validator) bool {
 	return v.PubKey.Equals(c2.PubKey) &&
 		bytes.Equal(v.Owner, c2.Owner) &&
 		v.PoolShares.Equal(c2.PoolShares) &&
 		v.DelegatorShares.Equal(c2.DelegatorShares) &&
 		v.Description == c2.Description &&
-		//v.BondHeight == c2.BondHeight &&
-		//v.BondIntraTxCounter == c2.BondIntraTxCounter && // counter is always changing
 		v.ProposerRewardPool.IsEqual(c2.ProposerRewardPool) &&
 		v.Commission.Equal(c2.Commission) &&
 		v.CommissionMax.Equal(c2.CommissionMax) &&
@@ -85,6 +84,7 @@ type Description struct {
 	Details  string `json:"details"`  // optional details
 }
 
+// NewDescription returns a new Description with the provided values.
 func NewDescription(moniker, identity, website, details string) Description {
 	return Description{
 		Moniker:  moniker,
@@ -94,20 +94,22 @@ func NewDescription(moniker, identity, website, details string) Description {
 	}
 }
 
-// update the description based on input
+// UpdateDescription updates the fields of a given description. An error is
+// returned if the resulting description contains an invalid length.
 func (d Description) UpdateDescription(d2 Description) (Description, sdk.Error) {
-	if d.Moniker == "[do-not-modify]" {
+	if d.Moniker == doNotModifyDescVal {
 		d2.Moniker = d.Moniker
 	}
-	if d.Identity == "[do-not-modify]" {
+	if d.Identity == doNotModifyDescVal {
 		d2.Identity = d.Identity
 	}
-	if d.Website == "[do-not-modify]" {
+	if d.Website == doNotModifyDescVal {
 		d2.Website = d.Website
 	}
-	if d.Details == "[do-not-modify]" {
+	if d.Details == doNotModifyDescVal {
 		d2.Details = d.Details
 	}
+
 	return Description{
 		Moniker:  d2.Moniker,
 		Identity: d2.Identity,
@@ -116,7 +118,7 @@ func (d Description) UpdateDescription(d2 Description) (Description, sdk.Error)
 	}.EnsureLength()
 }
 
-// ensure the length of the description
+// EnsureLength ensures the length of a validator's description.
 func (d Description) EnsureLength() (Description, sdk.Error) {
 	if len(d.Moniker) > 70 {
 		return d, ErrDescriptionLength(DefaultCodespace, "moniker", len(d.Moniker), 70)
@@ -130,10 +132,11 @@ func (d Description) EnsureLength() (Description, sdk.Error) {
 	if len(d.Details) > 280 {
 		return d, ErrDescriptionLength(DefaultCodespace, "details", len(d.Details), 280)
 	}
+
 	return d, nil
 }
 
-// abci validator from stake validator type
+// ABCIValidator returns an abci.Validator from a staked validator type.
 func (v Validator) ABCIValidator() abci.Validator {
 	return abci.Validator{
 		PubKey: tmtypes.TM2PB.PubKey(v.PubKey),
@@ -141,8 +144,8 @@ func (v Validator) ABCIValidator() abci.Validator {
 	}
 }
 
-// abci validator from stake validator type
+// ABCIValidatorZero returns an abci.Validator from a staked validator type
-// with zero power used for validator updates
+// with with zero power used for validator updates.
 func (v Validator) ABCIValidatorZero() abci.Validator {
 	return abci.Validator{
 		PubKey: tmtypes.TM2PB.PubKey(v.PubKey),
@@ -150,12 +153,13 @@ func (v Validator) ABCIValidatorZero() abci.Validator {
 	}
 }
 
-// abci validator from stake validator type
+// Status returns the validator's bond status inferred from the pool shares.
 func (v Validator) Status() sdk.BondStatus {
 	return v.PoolShares.Status
 }
 
-// update the location of the shares within a validator if its bond status has changed
+// UpdateStatus updates the location of the shares within a validator if it's
+// bond status has changed.
 func (v Validator) UpdateStatus(pool Pool, NewStatus sdk.BondStatus) (Validator, Pool) {
 	var tokens int64
 
@@ -173,7 +177,8 @@ func (v Validator) UpdateStatus(pool Pool, NewStatus sdk.BondStatus) (Validator,
 		pool, tokens = pool.removeSharesUnbonding(v.PoolShares.Amount)
 
 	case sdk.Bonded:
-		if NewStatus == sdk.Bonded { // return if nothing needs switching
+		if NewStatus == sdk.Bonded {
+			// Return if nothing needs switching
 			return v, pool
 		}
 		pool, tokens = pool.removeSharesBonded(v.PoolShares.Amount)
@@ -187,14 +192,16 @@ func (v Validator) UpdateStatus(pool Pool, NewStatus sdk.BondStatus) (Validator,
 	case sdk.Bonded:
 		pool, v.PoolShares = pool.addTokensBonded(tokens)
 	}
+
 	return v, pool
 }
 
-// Remove pool shares
+// RemovePoolShares removes pool shares from a validator. It returns
-// Returns corresponding tokens, which could be burned (e.g. when slashing
+// corresponding tokens, which could be burned (e.g. when slashing a validator)
-// a validator) or redistributed elsewhere
+// or redistributed elsewhere.
 func (v Validator) RemovePoolShares(pool Pool, poolShares sdk.Rat) (Validator, Pool, int64) {
 	var tokens int64
+
 	switch v.Status() {
 	case sdk.Unbonded:
 		pool, tokens = pool.removeSharesUnbonded(poolShares)
@@ -203,29 +210,33 @@ func (v Validator) RemovePoolShares(pool Pool, poolShares sdk.Rat) (Validator, P
 	case sdk.Bonded:
 		pool, tokens = pool.removeSharesBonded(poolShares)
 	}
+
 	v.PoolShares.Amount = v.PoolShares.Amount.Sub(poolShares)
 	return v, pool, tokens
 }
 
-// TODO remove should only be tokens
+// EquivalentBondedShares ...
-// get the power or potential power for a validator
+//
-// if bonded, the power is the BondedShares
+// TODO: Remove should only be tokens get the power or potential power for a
-// if not bonded, the power is the amount of bonded shares which the
+// validator if bonded, the power is the BondedShares if not bonded, the power
-//    the validator would have it was bonded
+// is the amount of bonded shares which the the validator would have it was
+// bonded.
 func (v Validator) EquivalentBondedShares(pool Pool) (eqBondedShares sdk.Rat) {
 	return v.PoolShares.ToBonded(pool).Amount
 }
 
 //_________________________________________________________________________________________________________
 
-// add tokens to a validator
+// AddTokensFromDel adds tokens to a validator
-func (v Validator) AddTokensFromDel(pool Pool,
+func (v Validator) AddTokensFromDel(pool Pool, amount int64) (Validator, Pool, sdk.Rat) {
-	amount int64) (validator2 Validator, p2 Pool, issuedDelegatorShares sdk.Rat) {
+	var (
+		poolShares             PoolShares
+		equivalentBondedShares sdk.Rat
+	)
 
-	exRate := v.DelegatorShareExRate(pool) // bshr/delshr
+	// bondedShare/delegatedShare
+	exRate := v.DelegatorShareExRate(pool)
 
-	var poolShares PoolShares
-	var equivalentBondedShares sdk.Rat
 	switch v.Status() {
 	case sdk.Unbonded:
 		pool, poolShares = pool.addTokensUnbonded(amount)
@@ -237,21 +248,24 @@ func (v Validator) AddTokensFromDel(pool Pool,
 	v.PoolShares.Amount = v.PoolShares.Amount.Add(poolShares.Amount)
 	equivalentBondedShares = poolShares.ToBonded(pool).Amount
 
-	issuedDelegatorShares = equivalentBondedShares.Quo(exRate) // bshr/(bshr/delshr) = delshr
+	// bondedShare/(bondedShare/delegatedShare) = delegatedShare
+	issuedDelegatorShares := equivalentBondedShares.Quo(exRate)
 	v.DelegatorShares = v.DelegatorShares.Add(issuedDelegatorShares)
 
 	return v, pool, issuedDelegatorShares
 }
 
-// remove delegator shares from a validator
+// RemoveDelShares removes delegator shares from a validator.
-// NOTE this function assumes the shares have already been updated for the validator status
+//
-func (v Validator) RemoveDelShares(pool Pool,
+// NOTE: This function assumes the shares have already been updated for the
-	delShares sdk.Rat) (validator2 Validator, p2 Pool, createdCoins int64) {
+// validator status.
-
+func (v Validator) RemoveDelShares(pool Pool, delShares sdk.Rat) (Validator, Pool, int64) {
 	amount := v.DelegatorShareExRate(pool).Mul(delShares)
 	eqBondedSharesToRemove := NewBondedShares(amount)
 	v.DelegatorShares = v.DelegatorShares.Sub(delShares)
 
+	var createdCoins int64
+
 	switch v.Status() {
 	case sdk.Unbonded:
 		unbondedShares := eqBondedSharesToRemove.ToUnbonded(pool).Amount
@@ -265,15 +279,17 @@ func (v Validator) RemoveDelShares(pool Pool,
 		pool, createdCoins = pool.removeSharesBonded(eqBondedSharesToRemove.Amount)
 		v.PoolShares.Amount = v.PoolShares.Amount.Sub(eqBondedSharesToRemove.Amount)
 	}
+
 	return v, pool, createdCoins
 }
 
-// get the exchange rate of tokens over delegator shares
+// DelegatorShareExRate gets the exchange rate of tokens over delegator shares.
 // UNITS: eq-val-bonded-shares/delegator-shares
 func (v Validator) DelegatorShareExRate(pool Pool) sdk.Rat {
 	if v.DelegatorShares.IsZero() {
 		return sdk.OneRat()
 	}
+
 	eqBondedShares := v.PoolShares.ToBonded(pool).Amount
 	return eqBondedShares.Quo(v.DelegatorShares)
 }
@@ -293,16 +309,20 @@ func (v Validator) GetPower() sdk.Rat           { return v.PoolShares.Bonded() }
 func (v Validator) GetDelegatorShares() sdk.Rat { return v.DelegatorShares }
 func (v Validator) GetBondHeight() int64        { return v.BondHeight }
 
-//Human Friendly pretty printer
+// HumanReadableString returns a human readable string representation of a
+// validator. An error is returned if the owner or the owner's public key
+// cannot be converted to Bech32 format.
 func (v Validator) HumanReadableString() (string, error) {
 	bechOwner, err := sdk.Bech32ifyAcc(v.Owner)
 	if err != nil {
 		return "", err
 	}
+
 	bechVal, err := sdk.Bech32ifyValPub(v.PubKey)
 	if err != nil {
 		return "", err
 	}
+
 	resp := "Validator \n"
 	resp += fmt.Sprintf("Owner: %s\n", bechOwner)
 	resp += fmt.Sprintf("Validator: %s\n", bechVal)

x/stake/types/validator_test.go

@@ -5,12 +5,89 @@ import (
 	"math/rand"
 	"testing"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
+	tmtypes "github.com/tendermint/tendermint/types"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+func TestValidatorEqual(t *testing.T) {
+	val1 := NewValidator(addr1, pk1, Description{})
+	val2 := NewValidator(addr1, pk1, Description{})
+
+	ok := val1.Equal(val2)
+	require.True(t, ok)
+
+	val2 = NewValidator(addr2, pk2, Description{})
+
+	ok = val1.Equal(val2)
+	require.False(t, ok)
+}
+
+func TestUpdateDescription(t *testing.T) {
+	d1 := Description{
+		Moniker:  doNotModifyDescVal,
+		Identity: doNotModifyDescVal,
+		Website:  doNotModifyDescVal,
+		Details:  doNotModifyDescVal,
+	}
+	d2 := Description{
+		Website: "https://validator.cosmos",
+		Details: "Test validator",
+	}
+
+	d, err := d1.UpdateDescription(d2)
+	require.Nil(t, err)
+	require.Equal(t, d, d1)
+}
+
+func TestABCIValidator(t *testing.T) {
+	val := NewValidator(addr1, pk1, Description{})
+
+	abciVal := val.ABCIValidator()
+	require.Equal(t, tmtypes.TM2PB.PubKey(val.PubKey), abciVal.PubKey)
+	require.Equal(t, val.PoolShares.Bonded().RoundInt64(), abciVal.Power)
+}
+
+func TestABCIValidatorZero(t *testing.T) {
+	val := NewValidator(addr1, pk1, Description{})
+
+	abciVal := val.ABCIValidatorZero()
+	require.Equal(t, tmtypes.TM2PB.PubKey(val.PubKey), abciVal.PubKey)
+	require.Equal(t, int64(0), abciVal.Power)
+}
+
+func TestRemovePoolShares(t *testing.T) {
+	pool := InitialPool()
+	pool.LooseTokens = 10
+
+	val := Validator{
+		Owner:           addr1,
+		PubKey:          pk1,
+		PoolShares:      NewBondedShares(sdk.NewRat(100)),
+		DelegatorShares: sdk.NewRat(100),
+	}
+
+	pool.BondedTokens = val.PoolShares.Bonded().RoundInt64()
+	pool.BondedShares = val.PoolShares.Bonded()
+
+	val, pool = val.UpdateStatus(pool, sdk.Bonded)
+	val, pool, tk := val.RemovePoolShares(pool, sdk.NewRat(10))
+	require.Equal(t, int64(90), val.PoolShares.Amount.RoundInt64())
+	require.Equal(t, int64(90), pool.BondedTokens)
+	require.Equal(t, int64(90), pool.BondedShares.RoundInt64())
+	require.Equal(t, int64(20), pool.LooseTokens)
+	require.Equal(t, int64(10), tk)
+
+	val, pool = val.UpdateStatus(pool, sdk.Unbonded)
+	val, pool, tk = val.RemovePoolShares(pool, sdk.NewRat(10))
+	require.Equal(t, int64(80), val.PoolShares.Amount.RoundInt64())
+	require.Equal(t, int64(0), pool.BondedTokens)
+	require.Equal(t, int64(0), pool.BondedShares.RoundInt64())
+	require.Equal(t, int64(30), pool.LooseTokens)
+	require.Equal(t, int64(10), tk)
+}
+
 func TestAddTokensValidatorBonded(t *testing.T) {
 	pool := InitialPool()
 	pool.LooseTokens = 10
@@ -230,3 +307,13 @@ func TestMultiValidatorIntegrationInvariants(t *testing.T) {
 		}
 	}
 }
+
+func TestHumanReadableString(t *testing.T) {
+	val := NewValidator(addr1, pk1, Description{})
+
+	// NOTE: Being that the validator's keypair is random, we cannot test the
+	// actual contents of the string.
+	valStr, err := val.HumanReadableString()
+	require.Nil(t, err)
+	require.NotEmpty(t, valStr)
+}