Merge PR #3554: x/auth and x/bank review results

Author: jackzampolin

client/rest/rest.go

@@ -237,7 +237,7 @@ func WriteGenerateStdTxResponse(
 		}
 	}
 
-	stdMsg, err := txBldr.Build(msgs)
+	stdMsg, err := txBldr.BuildSignMsg(msgs)
 	if err != nil {
 		WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 		return

client/utils/utils.go

@@ -294,7 +294,7 @@ func buildUnsignedStdTxOffline(txBldr authtxb.TxBuilder, cliCtx context.CLIConte
 		fmt.Fprintf(os.Stderr, "estimated gas = %v\n", txBldr.Gas())
 	}
 
-	stdSignMsg, err := txBldr.Build(msgs)
+	stdSignMsg, err := txBldr.BuildSignMsg(msgs)
 	if err != nil {
 		return
 	}

x/auth/account.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/tendermint/tendermint/crypto"
 
-	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -62,6 +61,7 @@ type VestingAccount interface {
 }
 
 // AccountDecoder unmarshals account bytes
+// TODO: Think about removing
 type AccountDecoder func(accountBytes []byte) (Account, error)
 
 //-----------------------------------------------------------------------------
@@ -90,32 +90,33 @@ func (acc BaseAccount) String() string {
 	}
 
 	return fmt.Sprintf(`Account:
-	Address:       %s
-	Pubkey:        %s
+  Address:       %s
+  Pubkey:        %s
   Coins:         %s
   AccountNumber: %d
   Sequence:      %d`,
 		acc.Address, pubkey, acc.Coins, acc.AccountNumber, acc.Sequence,
 	)
 }
 
-// Prototype function for BaseAccount
+// ProtoBaseAccount - a prototype function for BaseAccount
 func ProtoBaseAccount() Account {
 	return &BaseAccount{}
 }
 
+// NewBaseAccountWithAddress - returns a new base account with a given address
 func NewBaseAccountWithAddress(addr sdk.AccAddress) BaseAccount {
 	return BaseAccount{
 		Address: addr,
 	}
 }
 
-// Implements sdk.Account.
+// GetAddress - Implements sdk.Account.
 func (acc BaseAccount) GetAddress() sdk.AccAddress {
 	return acc.Address
 }
 
-// Implements sdk.Account.
+// SetAddress - Implements sdk.Account.
 func (acc *BaseAccount) SetAddress(addr sdk.AccAddress) error {
 	if len(acc.Address) != 0 {
 		return errors.New("cannot override BaseAccount address")
@@ -124,45 +125,45 @@ func (acc *BaseAccount) SetAddress(addr sdk.AccAddress) error {
 	return nil
 }
 
-// Implements sdk.Account.
+// GetPubKey - Implements sdk.Account.
 func (acc BaseAccount) GetPubKey() crypto.PubKey {
 	return acc.PubKey
 }
 
-// Implements sdk.Account.
+// SetPubKey - Implements sdk.Account.
 func (acc *BaseAccount) SetPubKey(pubKey crypto.PubKey) error {
 	acc.PubKey = pubKey
 	return nil
 }
 
-// Implements sdk.Account.
+// GetCoins - Implements sdk.Account.
 func (acc *BaseAccount) GetCoins() sdk.Coins {
 	return acc.Coins
 }
 
-// Implements sdk.Account.
+// SetCoins - Implements sdk.Account.
 func (acc *BaseAccount) SetCoins(coins sdk.Coins) error {
 	acc.Coins = coins
 	return nil
 }
 
-// Implements Account
+// GetAccountNumber - Implements Account
 func (acc *BaseAccount) GetAccountNumber() uint64 {
 	return acc.AccountNumber
 }
 
-// Implements Account
+// SetAccountNumber - Implements Account
 func (acc *BaseAccount) SetAccountNumber(accNumber uint64) error {
 	acc.AccountNumber = accNumber
 	return nil
 }
 
-// Implements sdk.Account.
+// GetSequence - Implements sdk.Account.
 func (acc *BaseAccount) GetSequence() uint64 {
 	return acc.Sequence
 }
 
-// Implements sdk.Account.
+// SetSequence - Implements sdk.Account.
 func (acc *BaseAccount) SetSequence(seq uint64) error {
 	acc.Sequence = seq
 	return nil
@@ -198,8 +199,8 @@ func (bva BaseVestingAccount) String() string {
 	}
 
 	return fmt.Sprintf(`Vesting Account:
-	Address:          %s
-	Pubkey:           %s
+  Address:          %s
+  Pubkey:           %s
   Coins:            %s
   AccountNumber:    %d
   Sequence:         %d
@@ -345,6 +346,7 @@ type ContinuousVestingAccount struct {
 	StartTime int64 // when the coins start to vest
 }
 
+// NewContinuousVestingAccount returns a new ContinuousVestingAccount
 func NewContinuousVestingAccount(
 	baseAcc *BaseAccount, StartTime, EndTime int64,
 ) *ContinuousVestingAccount {
@@ -369,8 +371,8 @@ func (cva ContinuousVestingAccount) String() string {
 	}
 
 	return fmt.Sprintf(`Continuous Vesting Account:
-	Address:          %s
-	Pubkey:           %s
+  Address:          %s
+  Pubkey:           %s
   Coins:            %s
   AccountNumber:    %d
   Sequence:         %d
@@ -454,6 +456,7 @@ type DelayedVestingAccount struct {
 	*BaseVestingAccount
 }
 
+// NewDelayedVestingAccount returns a DelayedVestingAccount
 func NewDelayedVestingAccount(baseAcc *BaseAccount, EndTime int64) *DelayedVestingAccount {
 	baseVestingAcc := &BaseVestingAccount{
 		BaseAccount:     baseAcc,
@@ -502,17 +505,3 @@ func (dva *DelayedVestingAccount) GetStartTime() int64 {
 func (dva *DelayedVestingAccount) GetEndTime() int64 {
 	return dva.EndTime
 }
-
-//-----------------------------------------------------------------------------
-// Codec
-
-// Most users shouldn't use this, but this comes in handy for tests.
-func RegisterBaseAccount(cdc *codec.Codec) {
-	cdc.RegisterInterface((*Account)(nil), nil)
-	cdc.RegisterInterface((*VestingAccount)(nil), nil)
-	cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil)
-	cdc.RegisterConcrete(&BaseVestingAccount{}, "cosmos-sdk/BaseVestingAccount", nil)
-	cdc.RegisterConcrete(&ContinuousVestingAccount{}, "cosmos-sdk/ContinuousVestingAccount", nil)
-	cdc.RegisterConcrete(&DelayedVestingAccount{}, "cosmos-sdk/DelayedVestingAccount", nil)
-	codec.RegisterCrypto(cdc)
-}

x/auth/ante.go

@@ -22,6 +22,7 @@ var (
 )
 
 func init() {
+	// This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation
 	bz, _ := hex.DecodeString("035AD6810A47F073553FF30D2FCC7E0D3B1C0B74B61A1AAA2582344037151E143A")
 	copy(simSecp256k1Pubkey[:], bz)
 }
@@ -192,9 +193,7 @@ func processSig(
 		return nil, sdk.ErrUnauthorized("signature verification failed").Result()
 	}
 
-	err = acc.SetSequence(acc.GetSequence() + 1)
-	if err != nil {
-		// Handle w/ #870
+	if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
 		panic(err)
 	}
 
@@ -327,7 +326,7 @@ func DeductFees(blockTime time.Time, acc Account, fee StdFee) (Account, sdk.Resu
 }
 
 // EnsureSufficientMempoolFees verifies that the given transaction has supplied
-// enough fees to cover a proposer's minimum fees. An result object is returned
+// enough fees to cover a proposer's minimum fees. A result object is returned
 // indicating success or failure.
 //
 // Contract: This should only be called during CheckTx as it cannot be part of

x/auth/client/txbuilder/txbuilder.go

@@ -174,11 +174,9 @@ func (bldr TxBuilder) WithAccountNumber(accnum uint64) TxBuilder {
 	return bldr
 }
 
-// Build builds a single message to be signed from a TxBuilder given a set of
+// BuildSignMsg builds a single message to be signed from a TxBuilder given a set of
 // messages. It returns an error if a fee is supplied but cannot be parsed.
-//
-// TODO: Should consider renaming to BuildSignMsg.
-func (bldr TxBuilder) Build(msgs []sdk.Msg) (StdSignMsg, error) {
+func (bldr TxBuilder) BuildSignMsg(msgs []sdk.Msg) (StdSignMsg, error) {
 	chainID := bldr.chainID
 	if chainID == "" {
 		return StdSignMsg{}, fmt.Errorf("chain ID required but not specified")
@@ -226,7 +224,7 @@ func (bldr TxBuilder) Sign(name, passphrase string, msg StdSignMsg) ([]byte, err
 // with the built message given a name, passphrase, and a set of
 // messages.
 func (bldr TxBuilder) BuildAndSign(name, passphrase string, msgs []sdk.Msg) ([]byte, error) {
-	msg, err := bldr.Build(msgs)
+	msg, err := bldr.BuildSignMsg(msgs)
 	if err != nil {
 		return nil, err
 	}
@@ -237,7 +235,7 @@ func (bldr TxBuilder) BuildAndSign(name, passphrase string, msgs []sdk.Msg) ([]b
 // BuildTxForSim creates a StdSignMsg and encodes a transaction with the
 // StdSignMsg with a single empty StdSignature for tx simulation.
 func (bldr TxBuilder) BuildTxForSim(msgs []sdk.Msg) ([]byte, error) {
-	signMsg, err := bldr.Build(msgs)
+	signMsg, err := bldr.BuildSignMsg(msgs)
 	if err != nil {
 		return nil, err
 	}

x/auth/client/txbuilder/txbuilder_test.go

@@ -94,7 +94,7 @@ func TestTxBuilderBuild(t *testing.T) {
 			tc.fields.ChainID, tc.fields.Memo, tc.fields.Fees, tc.fields.GasPrices,
 		)
 
-		got, err := bldr.Build(tc.msgs)
+		got, err := bldr.BuildSignMsg(tc.msgs)
 		require.Equal(t, tc.wantErr, (err != nil), "TxBuilder.Build() error = %v, wantErr %v, tc %d", err, tc.wantErr, i)
 		if !reflect.DeepEqual(got, tc.want) {
 			t.Errorf("TxBuilder.Build() = %v, want %v", got, tc.want)

x/auth/codec.go

@@ -4,7 +4,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 )
 
-// Register concrete types on codec codec for default AppAccount
+// RegisterCodec registers concrete types on the codec
 func RegisterCodec(cdc *codec.Codec) {
 	cdc.RegisterInterface((*Account)(nil), nil)
 	cdc.RegisterConcrete(&BaseAccount{}, "auth/Account", nil)
@@ -15,6 +15,17 @@ func RegisterCodec(cdc *codec.Codec) {
 	cdc.RegisterConcrete(StdTx{}, "auth/StdTx", nil)
 }
 
+// RegisterBaseAccount most users shouldn't use this, but this comes in handy for tests.
+func RegisterBaseAccount(cdc *codec.Codec) {
+	cdc.RegisterInterface((*Account)(nil), nil)
+	cdc.RegisterInterface((*VestingAccount)(nil), nil)
+	cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil)
+	cdc.RegisterConcrete(&BaseVestingAccount{}, "cosmos-sdk/BaseVestingAccount", nil)
+	cdc.RegisterConcrete(&ContinuousVestingAccount{}, "cosmos-sdk/ContinuousVestingAccount", nil)
+	cdc.RegisterConcrete(&DelayedVestingAccount{}, "cosmos-sdk/DelayedVestingAccount", nil)
+	codec.RegisterCrypto(cdc)
+}
+
 var msgCdc = codec.New()
 
 func init() {

x/auth/feekeeper.go

@@ -9,7 +9,7 @@ var (
 	collectedFeesKey = []byte("collectedFees")
 )
 
-// This FeeCollectionKeeper handles collection of fees in the anteHandler
+// FeeCollectionKeeper handles collection of fees in the anteHandler
 // and setting of MinFees for different fee tokens
 type FeeCollectionKeeper struct {
 
@@ -20,14 +20,15 @@ type FeeCollectionKeeper struct {
 	cdc *codec.Codec
 }
 
+// NewFeeCollectionKeeper returns a new FeeCollectionKeeper
 func NewFeeCollectionKeeper(cdc *codec.Codec, key sdk.StoreKey) FeeCollectionKeeper {
 	return FeeCollectionKeeper{
 		key: key,
 		cdc: cdc,
 	}
 }
 
-// retrieves the collected fee pool
+// GetCollectedFees - retrieves the collected fee pool
 func (fck FeeCollectionKeeper) GetCollectedFees(ctx sdk.Context) sdk.Coins {
 	store := ctx.KVStore(fck.key)
 	bz := store.Get(collectedFeesKey)
@@ -46,15 +47,15 @@ func (fck FeeCollectionKeeper) setCollectedFees(ctx sdk.Context, coins sdk.Coins
 	store.Set(collectedFeesKey, bz)
 }
 
-// add to the fee pool
+// AddCollectedFees - add to the fee pool
 func (fck FeeCollectionKeeper) AddCollectedFees(ctx sdk.Context, coins sdk.Coins) sdk.Coins {
 	newCoins := fck.GetCollectedFees(ctx).Plus(coins)
 	fck.setCollectedFees(ctx, newCoins)
 
 	return newCoins
 }
 
-// clear the fee pool
+// ClearCollectedFees - clear the fee pool
 func (fck FeeCollectionKeeper) ClearCollectedFees(ctx sdk.Context) {
 	fck.setCollectedFees(ctx, sdk.Coins{})
 }

x/auth/genesis.go

@@ -8,24 +8,24 @@ import (
 
 // GenesisState - all auth state that must be provided at genesis
 type GenesisState struct {
-	CollectedFees sdk.Coins `json:"collected_fees"` // collected fees
+	CollectedFees sdk.Coins `json:"collected_fees"`
 	Params        Params    `json:"params"`
 }
 
-// Create a new genesis state
+// NewGenesisState - Create a new genesis state
 func NewGenesisState(collectedFees sdk.Coins, params Params) GenesisState {
 	return GenesisState{
-		CollectedFees: collectedFees,
 		Params:        params,
+		CollectedFees: collectedFees,
 	}
 }
 
-// Return a default genesis state
+// DefaultGenesisState - Return a default genesis state
 func DefaultGenesisState() GenesisState {
 	return NewGenesisState(sdk.Coins{}, DefaultParams())
 }
 
-// Init store state from genesis data
+// InitGenesis - Init store state from genesis data
 func InitGenesis(ctx sdk.Context, ak AccountKeeper, fck FeeCollectionKeeper, data GenesisState) {
 	ak.SetParams(ctx, data.Params)
 	fck.setCollectedFees(ctx, data.CollectedFees)
@@ -57,6 +57,5 @@ func ValidateGenesis(data GenesisState) error {
 	if data.Params.TxSizeCostPerByte == 0 {
 		return fmt.Errorf("invalid tx size cost per byte: %d", data.Params.TxSizeCostPerByte)
 	}
-
 	return nil
 }