auth: Don't recalculate mempool fees for every msg signer, misc. cleanup

This PR begins improving the godocs for the auth module, and begins cleaning
up the Ante handler.

Additionally we previously calculated if the fee was sufficient for the tx
on every single signer. This is now refactored to be more efficient, and have
a better logical flow. No changelog entry as this is new to this release.

Author: ValarDragon

x/auth/account.go

@@ -8,8 +8,12 @@ import (
 	"github.com/tendermint/tendermint/crypto"
 )
 
-// Account is a standard account using a sequence number for replay protection
-// and a pubkey for authentication.
+// Account is an interface used to store coins at a given address within state.
+// It presumes a notion of sequence numbers for replay protection,
+// a notion of account numbers for replay protection for previously pruned accounts,
+// and a pubkey for authentication purposes.
+//
+// Many complex conditions can be used in the concrete struct which implements Account.
 type Account interface {
 	GetAddress() sdk.AccAddress
 	SetAddress(sdk.AccAddress) error // errors if already set.
@@ -35,9 +39,11 @@ type AccountDecoder func(accountBytes []byte) (Account, error)
 
 var _ Account = (*BaseAccount)(nil)
 
-// BaseAccount - base account structure.
-// Extend this by embedding this in your AppAccount.
-// See the examples/basecoin/types/account.go for an example.
+// BaseAccount - a base account structure.
+// This can be extended by embedding within in your AppAccount.
+// There are examples of this in: examples/basecoin/types/account.go.
+// However one doesn't have to use BaseAccount as long as your struct
+// implements Account.
 type BaseAccount struct {
 	Address       sdk.AccAddress `json:"address"`
 	Coins         sdk.Coins      `json:"coins"`
@@ -118,7 +124,7 @@ func (acc *BaseAccount) SetSequence(seq int64) error {
 //----------------------------------------
 // Wire
 
-// Most users shouldn't use this, but this comes handy for tests.
+// Most users shouldn't use this, but this comes in handy for tests.
 func RegisterBaseAccount(cdc *codec.Codec) {
 	cdc.RegisterInterface((*Account)(nil), nil)
 	cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil)

x/auth/ante.go

@@ -12,20 +12,18 @@ import (
 )
 
 const (
-	deductFeesCost        sdk.Gas = 10
-	memoCostPerByte       sdk.Gas = 1
-	ed25519VerifyCost             = 59
-	secp256k1VerifyCost           = 100
-	maxMemoCharacters             = 100
-	feeDeductionGasFactor         = 0.001
+	deductFeesCost      sdk.Gas = 10
+	memoCostPerByte     sdk.Gas = 1
+	ed25519VerifyCost           = 59
+	secp256k1VerifyCost         = 100
+	maxMemoCharacters           = 100
+	gasPrice                    = 0.001
 )
 
 // NewAnteHandler returns an AnteHandler that checks
 // and increments sequence numbers, checks signatures & account numbers,
 // and deducts fees from the first signer.
-// nolint: gocyclo
 func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
-
 	return func(
 		ctx sdk.Context, tx sdk.Tx, simulate bool,
 	) (newCtx sdk.Context, res sdk.Result, abort bool) {
@@ -36,13 +34,17 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
 			return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true
 		}
 
-		// set the gas meter
-		if simulate {
-			newCtx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
-		} else {
-			newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas))
+		// Ensure that the provided fees meet a minimum threshold for the validator, if this is a CheckTx.
+		// This is for mempool purposes, and thus is only ran on check tx.
+		if ctx.IsCheckTx() && !simulate {
+			res := ensureSufficientMempoolFees(ctx, stdTx)
+			if !res.IsOK() {
+				return newCtx, res, true
+			}
 		}
 
+		newCtx = setGasMeter(simulate, ctx, stdTx)
+
 		// AnteHandlers must have their own defer/recover in order
 		// for the BaseApp to know how much gas was used!
 		// This is because the GasMeter is created in the AnteHandler,
@@ -66,52 +68,37 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
 		if err != nil {
 			return newCtx, err.Result(), true
 		}
+		// charge gas for the memo
+		newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo")
 
-		sigs := stdTx.GetSignatures() // When simulating, this would just be a 0-length slice.
+		// stdSigs contains the sequence number, account number, and signatures
+		stdSigs := stdTx.GetSignatures() // When simulating, this would just be a 0-length slice.
 		signerAddrs := stdTx.GetSigners()
-		msgs := tx.GetMsgs()
 
-		// charge gas for the memo
-		newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo")
+		// create the list of all sign bytes
+		signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, stdSigs)
 
 		// Get the sign bytes (requires all account & sequence numbers and the fee)
-		sequences := make([]int64, len(sigs))
-		accNums := make([]int64, len(sigs))
-		for i := 0; i < len(sigs); i++ {
-			sequences[i] = sigs[i].Sequence
-			accNums[i] = sigs[i].AccountNumber
-		}
-		fee := stdTx.Fee
-
 		// Check sig and nonce and collect signer accounts.
 		var signerAccs = make([]Account, len(signerAddrs))
-		for i := 0; i < len(sigs); i++ {
-			signerAddr, sig := signerAddrs[i], sigs[i]
+		for i := 0; i < len(stdSigs); i++ {
+			signerAddr, sig := signerAddrs[i], stdSigs[i]
 
 			// check signature, return account with incremented nonce
-			signBytes := StdSignBytes(newCtx.ChainID(), accNums[i], sequences[i], fee, msgs, stdTx.GetMemo())
-			signerAcc, res := processSig(newCtx, am, signerAddr, sig, signBytes, simulate)
+			signerAcc, res := processSig(newCtx, am, signerAddr, sig, signBytesList[i], simulate)
 			if !res.IsOK() {
 				return newCtx, res, true
 			}
 
-			requiredFees := adjustFeesByGas(ctx.MinimumFees(), fee.Gas)
-			// fees must be greater than the minimum set by the validator adjusted by gas
-			if ctx.IsCheckTx() && !simulate && !ctx.MinimumFees().IsZero() && fee.Amount.IsLT(requiredFees) {
-				// validators reject any tx from the mempool with less than the minimum fee per gas * gas factor
-				return newCtx, sdk.ErrInsufficientFee(fmt.Sprintf(
-					"insufficient fee, got: %q required: %q", fee.Amount, requiredFees)).Result(), true
-			}
-
 			// first sig pays the fees
-			// Can this function be moved outside of the loop?
-			if i == 0 && !fee.Amount.IsZero() {
+			// TODO: move outside of the loop
+			if i == 0 && !stdTx.Fee.Amount.IsZero() {
 				newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees")
-				signerAcc, res = deductFees(signerAcc, fee)
+				signerAcc, res = deductFees(signerAcc, stdTx.Fee)
 				if !res.IsOK() {
 					return newCtx, res, true
 				}
-				fck.addCollectedFees(newCtx, fee.Amount)
+				fck.addCollectedFees(newCtx, stdTx.Fee.Amount)
 			}
 
 			// Save the account.
@@ -153,6 +140,7 @@ func validateBasic(tx StdTx) (err sdk.Error) {
 
 // verify the signature and increment the sequence.
 // if the account doesn't have a pubkey, set it.
+// TODO: Change this function to already take in the account
 func processSig(
 	ctx sdk.Context, am AccountMapper,
 	addr sdk.AccAddress, sig StdSignature, signBytes []byte, simulate bool) (
@@ -245,8 +233,9 @@ func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) {
 }
 
 func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins {
-	gasCost := int64(float64(gas) * feeDeductionGasFactor)
+	gasCost := int64(float64(gas) * gasPrice)
 	gasFees := make(sdk.Coins, len(fees))
+	// TODO: Make this not price all coins in the same way
 	for i := 0; i < len(fees); i++ {
 		gasFees[i] = sdk.NewInt64Coin(fees[i].Denom, gasCost)
 	}
@@ -273,5 +262,38 @@ func deductFees(acc Account, fee StdFee) (Account, sdk.Result) {
 	return acc, sdk.Result{}
 }
 
+func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result {
+	// currently we use a very primitive gas pricing model with a constant gasPrice.
+	// adjustFeesByGas handles calculating the amount of fees required based on the provided gas.
+	// TODO: Make the gasPrice not a constant, and account for tx size.
+	requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas)
+
+	if !ctx.MinimumFees().IsZero() && stdTx.Fee.Amount.IsLT(requiredFees) {
+		// validators reject any tx from the mempool with less than the minimum fee per gas * gas factor
+		return sdk.ErrInsufficientFee(fmt.Sprintf(
+			"insufficient fee, got: %q required: %q", stdTx.Fee.Amount, requiredFees)).Result()
+	}
+	return sdk.Result{}
+}
+
+func setGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context {
+	// set the gas meter
+	if simulate {
+		return ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
+	}
+	return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas))
+}
+
+func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) (
+	signatureBytesList [][]byte) {
+	signatureBytesList = make([][]byte, len(stdSigs))
+	for i := 0; i < len(stdSigs); i++ {
+		signatureBytesList[i] = StdSignBytes(chainID,
+			stdSigs[i].AccountNumber, stdSigs[i].Sequence,
+			stdTx.Fee, stdTx.Msgs, stdTx.Memo)
+	}
+	return
+}
+
 // BurnFeeHandler burns all fees (decreasing total supply)
 func BurnFeeHandler(_ sdk.Context, _ sdk.Tx, _ sdk.Coins) {}