From a768543d92a1a555338056f354a51af92dee321b Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Sat, 7 Jul 2018 18:59:06 -0700
Subject: [PATCH 01/11] tools: Add code complexity linter, gocyclo

Gocyclo is a code complexity linter. It uses cyclomatic complexity.
Cyclomatic complexity essentially measures the number of different
paths code could go through. (The conditional in a for loop counts
as adding one path) It looks at this on a per-function level. The
idea that this would be enforcing is that if there are too many
different paths code can go through in a function, it needs to be
better split up. (A function with too many code paths is hard to
reason about)

The complexity which we want the linter to start failing on is
configurable. The default is 10. Change the "Cyclo" parameter in
`tools/gometalinter.json` to try other values.
---
 CHANGELOG.md            |  1 +
 tools/Makefile          | 15 +++++++++++++++
 tools/gometalinter.json |  5 +++--
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e63d01c4b43b..48ddab70f810 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -70,6 +70,7 @@ FEATURES
   * ineffassign
   * errcheck
   * unparam
+  * gocyclo
 * [tools] Add `make format` command to automate fixing misspell and gofmt errors.
 * [server] Default config now creates a profiler at port 6060, and increase p2p send/recv rates
 * [tests] Add WaitForNextNBlocksTM helper method
diff --git a/tools/Makefile b/tools/Makefile
index 195e67aa5228..d58f52d1b184 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -12,6 +12,7 @@ INEFFASSIGN = github.com/gordonklaus/ineffassign
 MISSPELL = github.com/client9/misspell/cmd/misspell
 ERRCHECK = github.com/kisielk/errcheck
 UNPARAM = mvdan.cc/unparam
+GOCYCLO = github.com/alecthomas/gocyclo
 
 DEP_CHECK := $(shell command -v dep 2> /dev/null)
 GOLINT_CHECK := $(shell command -v golint 2> /dev/null)
@@ -21,6 +22,7 @@ INEFFASSIGN_CHECK := $(shell command -v ineffassign 2> /dev/null)
 MISSPELL_CHECK := $(shell command -v misspell 2> /dev/null)
 ERRCHECK_CHECK := $(shell command -v errcheck 2> /dev/null)
 UNPARAM_CHECK := $(shell command -v unparam 2> /dev/null)
+GOCYCLO_CHECK := $(shell command -v gocyclo 2> /dev/null)
 
 check_tools:
 ifndef DEP_CHECK
@@ -63,6 +65,11 @@ ifndef UNPARAM_CHECK
 else
 	@echo "Found unparam in path."
 endif
+ifndef GOCYCLO_CHECK
+	@echo "No gocyclo in path.  Install with 'make get_tools'."
+else
+	@echo "Found gocyclo in path."
+endif
 
 get_tools:
 ifdef DEP_CHECK
@@ -113,6 +120,12 @@ else
 	@echo "Installing unparam"
 	go get -v $(UNPARAM)
 endif
+ifdef GOYCLO_CHECK
+	@echo "goyclo is already installed.  Run 'make update_tools' to update."
+else
+	@echo "Installing goyclo"
+	go get -v $(GOCYCLO)
+endif
 
 update_tools:
 	@echo "Updating dep"
@@ -131,6 +144,8 @@ update_tools:
 	go get -u -v $(ERRCHECK)
 	@echo "Updating unparam"
 	go get -u -v $(UNPARAM)
+	@echo "Updating goyclo"
+	go get -u -v $(GOCYCLO)
 
 # To avoid unintended conflicts with file names, always add to .PHONY
 # unless there is a reason not to.
diff --git a/tools/gometalinter.json b/tools/gometalinter.json
index a6c74eebbbab..32ae434c4947 100644
--- a/tools/gometalinter.json
+++ b/tools/gometalinter.json
@@ -2,7 +2,8 @@
     "Linters": {
         "vet": "go tool vet -composites=false :PATH:LINE:MESSAGE"
    },
-    "Enable": ["golint", "vet", "ineffassign", "unparam", "unconvert", "misspell"],
+    "Enable": ["golint", "vet", "ineffassign", "unparam", "unconvert", "misspell", "gocyclo"],
     "Deadline": "500s",
-    "Vendor": true
+    "Vendor": true,
+    "Cyclo": 10
 }
\ No newline at end of file

From 2b5ccdbf879009d96a02389e7b863c9126d6cccf Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Sat, 7 Jul 2018 19:34:46 -0700
Subject: [PATCH 02/11] Reduce code complexity in AnteHandler, and
 GetExternalIP

---
 server/util.go | 36 +++++++++++++++--------
 x/auth/ante.go | 79 ++++++++++++++++++++++++++------------------------
 2 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/server/util.go b/server/util.go
index 0f1816b16664..1e6ed06c9812 100644
--- a/server/util.go
+++ b/server/util.go
@@ -149,24 +149,15 @@ func externalIP() (string, error) {
 		return "", err
 	}
 	for _, iface := range ifaces {
-		if iface.Flags&net.FlagUp == 0 {
-			continue // interface down
-		}
-		if iface.Flags&net.FlagLoopback != 0 {
-			continue // loopback interface
+		if skipInterface(iface) {
+			continue
 		}
 		addrs, err := iface.Addrs()
 		if err != nil {
 			return "", err
 		}
 		for _, addr := range addrs {
-			var ip net.IP
-			switch v := addr.(type) {
-			case *net.IPNet:
-				ip = v.IP
-			case *net.IPAddr:
-				ip = v.IP
-			}
+			ip := addrToIP(addr)
 			if ip == nil || ip.IsLoopback() {
 				continue
 			}
@@ -179,3 +170,24 @@ func externalIP() (string, error) {
 	}
 	return "", errors.New("are you connected to the network?")
 }
+
+func skipInterface(iface net.Interface) bool {
+	if iface.Flags&net.FlagUp == 0 {
+		return true // interface down
+	}
+	if iface.Flags&net.FlagLoopback != 0 {
+		return true // loopback interface
+	}
+	return false
+}
+
+func addrToIP(addr net.Addr) net.IP {
+	var ip net.IP
+	switch v := addr.(type) {
+	case *net.IPNet:
+		ip = v.IP
+	case *net.IPAddr:
+		ip = v.IP
+	}
+	return ip
+}
diff --git a/x/auth/ante.go b/x/auth/ante.go
index ff3145d4565a..c71b17474371 100644
--- a/x/auth/ante.go
+++ b/x/auth/ante.go
@@ -29,45 +29,26 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
 			return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true
 		}
 
-		// Assert that there are signatures.
-		var sigs = stdTx.GetSignatures()
-		if len(sigs) == 0 {
-			return ctx,
-				sdk.ErrUnauthorized("no signers").Result(),
-				true
+		err := validateBasic(stdTx)
+		if err != nil {
+			return ctx, err.Result(), true
 		}
 
-		memo := stdTx.GetMemo()
-
-		if len(memo) > maxMemoCharacters {
-			return ctx,
-				sdk.ErrMemoTooLarge(fmt.Sprintf("maximum number of characters is %d but received %d characters", maxMemoCharacters, len(memo))).Result(),
-				true
-		}
+		sigs := stdTx.GetSignatures()
+		signerAddrs := stdTx.GetSigners()
+		msgs := tx.GetMsgs()
 
 		// set the gas meter
 		ctx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas))
 
 		// charge gas for the memo
-		ctx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(memo)), "memo")
-
-		msgs := tx.GetMsgs()
-
-		// Assert that number of signatures is correct.
-		var signerAddrs = stdTx.GetSigners()
-		if len(sigs) != len(signerAddrs) {
-			return ctx,
-				sdk.ErrUnauthorized("wrong number of signers").Result(),
-				true
-		}
+		ctx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo")
 
 		// Get the sign bytes (requires all account & sequence numbers and the fee)
-		sequences := make([]int64, len(signerAddrs))
-		for i := 0; i < len(signerAddrs); i++ {
+		sequences := make([]int64, len(sigs))
+		accNums := make([]int64, len(sigs))
+		for i := 0; i < len(sigs); i++ {
 			sequences[i] = sigs[i].Sequence
-		}
-		accNums := make([]int64, len(signerAddrs))
-		for i := 0; i < len(signerAddrs); i++ {
 			accNums[i] = sigs[i].AccountNumber
 		}
 		fee := stdTx.Fee
@@ -88,16 +69,15 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
 			}
 
 			// first sig pays the fees
-			if i == 0 {
-				// TODO: min fee
-				if !fee.Amount.IsZero() {
-					ctx.GasMeter().ConsumeGas(deductFeesCost, "deductFees")
-					signerAcc, res = deductFees(signerAcc, fee)
-					if !res.IsOK() {
-						return ctx, res, true
-					}
-					fck.addCollectedFees(ctx, fee.Amount)
+			// TODO: Add min fees
+			// Can this function be moved outside of the loop?
+			if i == 0 && !fee.Amount.IsZero() {
+				ctx.GasMeter().ConsumeGas(deductFeesCost, "deductFees")
+				signerAcc, res = deductFees(signerAcc, fee)
+				if !res.IsOK() {
+					return ctx, res, true
 				}
+				fck.addCollectedFees(ctx, fee.Amount)
 			}
 
 			// Save the account.
@@ -114,6 +94,29 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler {
 	}
 }
 
+// Validate the transaction based on things that don't depend on the context
+func validateBasic(tx StdTx) (err sdk.Error) {
+	// Assert that there are signatures.
+	sigs := tx.GetSignatures()
+	if len(sigs) == 0 {
+		return sdk.ErrUnauthorized("no signers")
+	}
+
+	// Assert that number of signatures is correct.
+	var signerAddrs = tx.GetSigners()
+	if len(sigs) != len(signerAddrs) {
+		return sdk.ErrUnauthorized("wrong number of signers")
+	}
+
+	memo := tx.GetMemo()
+	if len(memo) > maxMemoCharacters {
+		return sdk.ErrMemoTooLarge(
+			fmt.Sprintf("maximum number of characters is %d but received %d characters",
+				maxMemoCharacters, len(memo)))
+	}
+	return nil
+}
+
 // verify the signature and increment the sequence.
 // if the account doesn't have a pubkey, set it.
 func processSig(

From 0c5358c2676b0a2be66540d22411a29f72e28ed6 Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 13:54:28 -0700
Subject: [PATCH 03/11] Continue reducing code complexity: * Adds a Min
 function to Int, and uses that in the slash function * Adds a getHeight
 helper function to iavlstore * Adds a splitPath function to baseapp * Changes
 cyclo param from 10 to 11

---
 baseapp/baseapp.go      | 13 +++++++++----
 store/iavlstore.go      | 33 ++++++++++++++++++++-------------
 tools/gometalinter.json |  2 +-
 types/int.go            | 17 +++++++++++++++++
 x/stake/keeper/slash.go | 10 ++--------
 5 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 9c7ce727fa45..f4f213279cf9 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -324,14 +324,19 @@ func (app *BaseApp) FilterPeerByPubKey(info string) abci.ResponseQuery {
 	return abci.ResponseQuery{}
 }
 
-// Implements ABCI.
-// Delegates to CommitMultiStore if it implements Queryable
-func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
-	path := strings.Split(req.Path, "/")
+func splitPath(requestPath string) (path []string) {
+	path = strings.Split(requestPath, "/")
 	// first element is empty string
 	if len(path) > 0 && path[0] == "" {
 		path = path[1:]
 	}
+	return path
+}
+
+// Implements ABCI.
+// Delegates to CommitMultiStore if it implements Queryable
+func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
+	path := splitPath(req.Path)
 	// "/app" prefix for special application queries
 	if len(path) >= 2 && path[0] == "app" {
 		var result sdk.Result
diff --git a/store/iavlstore.go b/store/iavlstore.go
index e5d509572849..d05c867c2ebb 100644
--- a/store/iavlstore.go
+++ b/store/iavlstore.go
@@ -153,6 +153,20 @@ func (st *iavlStore) ReverseIterator(start, end []byte) Iterator {
 	return newIAVLIterator(st.tree.Tree(), start, end, false)
 }
 
+// Handle gatest the latest height, if height is 0
+func getHeight(tree *iavl.VersionedTree, req abci.RequestQuery) int64 {
+	height := req.Height
+	if height == 0 {
+		latest := tree.Version64()
+		if tree.VersionExists(latest - 1) {
+			height = latest - 1
+		} else {
+			height = latest
+		}
+	}
+	return height
+}
+
 // Query implements ABCI interface, allows queries
 //
 // by default we will return from (latest height -1),
@@ -167,24 +181,17 @@ func (st *iavlStore) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 	}
 
 	tree := st.tree
-	height := req.Height
-	if height == 0 {
-		latest := tree.Version64()
-		if tree.VersionExists(latest - 1) {
-			height = latest - 1
-		} else {
-			height = latest
-		}
-	}
-	// store the height we chose in the response
-	res.Height = height
+
+	// store the height we chose in the response, with 0 being changed to the
+	// latest height
+	res.Height = getHeight(tree, req)
 
 	switch req.Path {
 	case "/store", "/key": // Get by key
 		key := req.Data // Data holds the key bytes
 		res.Key = key
 		if req.Prove {
-			value, proof, err := tree.GetVersionedWithProof(key, height)
+			value, proof, err := tree.GetVersionedWithProof(key, res.Height)
 			if err != nil {
 				res.Log = err.Error()
 				break
@@ -198,7 +205,7 @@ func (st *iavlStore) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 			}
 			res.Proof = p
 		} else {
-			_, res.Value = tree.GetVersioned(key, height)
+			_, res.Value = tree.GetVersioned(key, res.Height)
 		}
 	case "/subspace":
 		subspace := req.Data
diff --git a/tools/gometalinter.json b/tools/gometalinter.json
index 32ae434c4947..124e28c147fe 100644
--- a/tools/gometalinter.json
+++ b/tools/gometalinter.json
@@ -5,5 +5,5 @@
     "Enable": ["golint", "vet", "ineffassign", "unparam", "unconvert", "misspell", "gocyclo"],
     "Deadline": "500s",
     "Vendor": true,
-    "Cyclo": 10
+    "Cyclo": 11
 }
\ No newline at end of file
diff --git a/types/int.go b/types/int.go
index d04c6a80cd72..0227203cdc31 100644
--- a/types/int.go
+++ b/types/int.go
@@ -37,6 +37,13 @@ func mod(i *big.Int, i2 *big.Int) *big.Int { return new(big.Int).Mod(i, i2) }
 
 func neg(i *big.Int) *big.Int { return new(big.Int).Neg(i) }
 
+func min(i *big.Int, i2 *big.Int) *big.Int {
+	if i.Cmp(i2) == 1 {
+		return new(big.Int).Set(i2)
+	}
+	return new(big.Int).Set(i)
+}
+
 // MarshalAmino for custom encoding scheme
 func marshalAmino(i *big.Int) (string, error) {
 	bz, err := i.MarshalText()
@@ -227,6 +234,11 @@ func (i Int) Neg() (res Int) {
 	return Int{neg(i.i)}
 }
 
+// Return the minimum of the ints
+func MinInt(i1, i2 Int) Int {
+	return Int{min(i1.BigInt(), i2.BigInt())}
+}
+
 func (i Int) String() string {
 	return i.i.String()
 }
@@ -419,6 +431,11 @@ func (i Uint) DivRaw(i2 uint64) Uint {
 	return i.Div(NewUint(i2))
 }
 
+// Return the minimum of the Uints
+func MinUint(i1, i2 Uint) Uint {
+	return Uint{min(i1.BigInt(), i2.BigInt())}
+}
+
 // MarshalAmino defines custom encoding scheme
 func (i Uint) MarshalAmino() (string, error) {
 	if i.i == nil { // Necessary since default Uint initialization has i.i as nil
diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go
index f194d656cbf2..a2080eab93bf 100644
--- a/x/stake/keeper/slash.go
+++ b/x/stake/keeper/slash.go
@@ -81,10 +81,7 @@ func (k Keeper) Slash(ctx sdk.Context, pubkey crypto.PubKey, infractionHeight in
 	}
 
 	// Cannot decrease balance below zero
-	sharesToRemove := remainingSlashAmount
-	if sharesToRemove.GT(validator.PoolShares.Amount.RoundInt()) {
-		sharesToRemove = validator.PoolShares.Amount.RoundInt()
-	}
+	sharesToRemove := sdk.MinInt(remainingSlashAmount, validator.PoolShares.Amount.RoundInt())
 
 	// Get the current pool
 	pool := k.GetPool(ctx)
@@ -163,10 +160,7 @@ func (k Keeper) slashUnbondingDelegation(ctx sdk.Context, unbondingDelegation ty
 	// Possible since the unbonding delegation may already
 	// have been slashed, and slash amounts are calculated
 	// according to stake held at time of infraction
-	unbondingSlashAmount := slashAmount
-	if unbondingSlashAmount.GT(unbondingDelegation.Balance.Amount) {
-		unbondingSlashAmount = unbondingDelegation.Balance.Amount
-	}
+	unbondingSlashAmount := sdk.MinInt(slashAmount, unbondingDelegation.Balance.Amount)
 
 	// Update unbonding delegation if necessary
 	if !unbondingSlashAmount.IsZero() {

From 1433b61a92531756fb420382306647d1d0ea5d30 Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 14:11:51 -0700
Subject: [PATCH 04/11] Missed a min in slashing

---
 x/stake/keeper/slash.go | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go
index a2080eab93bf..44bc2aade609 100644
--- a/x/stake/keeper/slash.go
+++ b/x/stake/keeper/slash.go
@@ -202,10 +202,7 @@ func (k Keeper) slashRedelegation(ctx sdk.Context, validator types.Validator, re
 	// Possible since the redelegation may already
 	// have been slashed, and slash amounts are calculated
 	// according to stake held at time of infraction
-	redelegationSlashAmount := slashAmount
-	if redelegationSlashAmount.GT(redelegation.Balance.Amount) {
-		redelegationSlashAmount = redelegation.Balance.Amount
-	}
+	redelegationSlashAmount := sdk.MinInt(slashAmount, redelegation.Balance.Amount)
 
 	// Update redelegation if necessary
 	if !redelegationSlashAmount.IsZero() {

From 2a419a3192e4b44cca4ed244d80d9f32d84f6705 Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 14:21:29 -0700
Subject: [PATCH 05/11] add helper to UpdateBondedValidatorsFull

---
 x/stake/keeper/inflation_test.go | 1 +
 x/stake/keeper/validator.go      | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/x/stake/keeper/inflation_test.go b/x/stake/keeper/inflation_test.go
index 2fee8154a007..28efc0c59b8e 100644
--- a/x/stake/keeper/inflation_test.go
+++ b/x/stake/keeper/inflation_test.go
@@ -342,6 +342,7 @@ func checkValidatorSetup(t *testing.T, pool types.Pool, initialTotalTokens, init
 }
 
 // Checks that The inflation will correctly increase or decrease after an update to the pool
+// nolint: gocyclo
 func checkInflation(t *testing.T, pool types.Pool, previousInflation, updatedInflation sdk.Rat, msg string) {
 	inflationChange := updatedInflation.Sub(previousInflation)
 
diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go
index 233c7b6e87c0..e8713c04d54c 100644
--- a/x/stake/keeper/validator.go
+++ b/x/stake/keeper/validator.go
@@ -422,6 +422,11 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) {
 	iterator.Close()
 
 	// perform the actual kicks
+	kickOutValidators(k, ctx, toKickOut)
+	return
+}
+
+func kickOutValidators(k Keeper, ctx sdk.Context, toKickOut map[string]byte) {
 	for key := range toKickOut {
 		ownerAddr := []byte(key)
 		validator, found := k.GetValidator(ctx, ownerAddr)
@@ -430,7 +435,6 @@ func (k Keeper) UpdateBondedValidatorsFull(ctx sdk.Context) {
 		}
 		k.unbondValidator(ctx, validator)
 	}
-	return
 }
 
 // perform all the store operations for when a validator status becomes unbonded

From e906272ca075c35937aff99b47274a6808a3215f Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 14:44:01 -0700
Subject: [PATCH 06/11] Reduce complexity of baseapp query

---
 baseapp/baseapp.go        | 50 ++++++++++++++++++++++++++++++---------
 x/ibc/client/cli/relay.go |  2 ++
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index f4f213279cf9..0316663d35af 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -337,8 +337,26 @@ func splitPath(requestPath string) (path []string) {
 // Delegates to CommitMultiStore if it implements Queryable
 func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 	path := splitPath(req.Path)
+	if len(path) == 0 {
+		msg := "no query path provided"
+		return sdk.ErrUnknownRequest(msg).QueryResult()
+	}
+	switch path[0] {
 	// "/app" prefix for special application queries
-	if len(path) >= 2 && path[0] == "app" {
+	case "app":
+		return handleQueryApp(app, path, req)
+	case "store":
+		return handleQueryStore(app, path, req)
+	case "p2p":
+		return handleQueryP2P(app, path, req)
+	}
+
+	msg := "unknown query path"
+	return sdk.ErrUnknownRequest(msg).QueryResult()
+}
+
+func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
+	if len(path) >= 2 {
 		var result sdk.Result
 		switch path[1] {
 		case "simulate":
@@ -363,18 +381,24 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 			Value: value,
 		}
 	}
+	msg := "Expected second parameter to be either simulate or version, neither was present"
+	return sdk.ErrUnknownRequest(msg).QueryResult()
+}
+
+func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
 	// "/store" prefix for store queries
-	if len(path) >= 1 && path[0] == "store" {
-		queryable, ok := app.cms.(sdk.Queryable)
-		if !ok {
-			msg := "multistore doesn't support queries"
-			return sdk.ErrUnknownRequest(msg).QueryResult()
-		}
-		req.Path = "/" + strings.Join(path[1:], "/")
-		return queryable.Query(req)
+	queryable, ok := app.cms.(sdk.Queryable)
+	if !ok {
+		msg := "multistore doesn't support queries"
+		return sdk.ErrUnknownRequest(msg).QueryResult()
 	}
+	req.Path = "/" + strings.Join(path[1:], "/")
+	return queryable.Query(req)
+}
+
+func handleQueryP2P(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) {
 	// "/p2p" prefix for p2p queries
-	if len(path) >= 4 && path[0] == "p2p" {
+	if len(path) >= 4 {
 		if path[1] == "filter" {
 			if path[2] == "addr" {
 				return app.FilterPeerByAddrPort(path[3])
@@ -384,9 +408,13 @@ func (app *BaseApp) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
 				// NOTE: this changed in tendermint and we didn't notice...
 				return app.FilterPeerByPubKey(path[3])
 			}
+		} else {
+			msg := "Expected second parameter to be filter"
+			return sdk.ErrUnknownRequest(msg).QueryResult()
 		}
 	}
-	msg := "unknown query path"
+
+	msg := "Expected path is p2p filter <addr|pubkey> <parameter>"
 	return sdk.ErrUnknownRequest(msg).QueryResult()
 }
 
diff --git a/x/ibc/client/cli/relay.go b/x/ibc/client/cli/relay.go
index d434ff35a44c..1c5a61a18f1f 100644
--- a/x/ibc/client/cli/relay.go
+++ b/x/ibc/client/cli/relay.go
@@ -86,6 +86,8 @@ func (c relayCommander) runIBCRelay(cmd *cobra.Command, args []string) {
 	c.loop(fromChainID, fromChainNode, toChainID, toChainNode)
 }
 
+// This is nolinted as someone is in the process of refactoring this to remove the goto
+// nolint: gocyclo
 func (c relayCommander) loop(fromChainID, fromChainNode, toChainID,
 	toChainNode string) {
 

From 592419c83a1b804eb925c31c628c16d306f5df2d Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 15:00:55 -0700
Subject: [PATCH 07/11] Reduce code complexity of testnet command

---
 server/testnet.go | 69 ++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 30 deletions(-)

diff --git a/server/testnet.go b/server/testnet.go
index d102b87ccb7e..d7e4ec9ac7f0 100644
--- a/server/testnet.go
+++ b/server/testnet.go
@@ -60,10 +60,11 @@ Example:
 }
 
 func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) error {
-
 	outDir := viper.GetString(outputDir)
+	numValidators := viper.GetInt(nValidators)
+
 	// Generate private key, node ID, initial transaction
-	for i := 0; i < viper.GetInt(nValidators); i++ {
+	for i := 0; i < numValidators; i++ {
 		nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
 		nodeDir := filepath.Join(outDir, nodeDirName, "gaiad")
 		clientDir := filepath.Join(outDir, nodeDirName, "gaiacli")
@@ -83,18 +84,9 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 		}
 
 		config.Moniker = nodeDirName
-
-		ip := viper.GetString(startingIPAddress)
-		if len(ip) == 0 {
-			ip, err = externalIP()
-			if err != nil {
-				return err
-			}
-		} else {
-			ip, err = calculateIP(ip, i)
-			if err != nil {
-				return err
-			}
+		ip, err := getIP(i)
+		if err != nil {
+			return err
 		}
 
 		genTxConfig := gc.GenTx{
@@ -112,35 +104,22 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 
 		// Save private key seed words
 		name := fmt.Sprintf("%v.json", "key_seed")
-		writePath := filepath.Join(clientDir)
-		file := filepath.Join(writePath, name)
-		err = cmn.EnsureDir(writePath, 0700)
-		if err != nil {
-			return err
-		}
-		err = cmn.WriteFile(file, cliPrint, 0600)
+		err = writeFile(name, clientDir, cliPrint)
 		if err != nil {
 			return err
 		}
 
 		// Gather gentxs folder
 		name = fmt.Sprintf("%v.json", nodeDirName)
-		writePath = filepath.Join(gentxsDir)
-		file = filepath.Join(writePath, name)
-		err = cmn.EnsureDir(writePath, 0700)
-		if err != nil {
-			return err
-		}
-		err = cmn.WriteFile(file, genTxFile, 0644)
+		err = writeFile(name, gentxsDir, genTxFile)
 		if err != nil {
 			return err
 		}
-
 	}
 
 	// Generate genesis.json and config.toml
 	chainID := "chain-" + cmn.RandStr(6)
-	for i := 0; i < viper.GetInt(nValidators); i++ {
+	for i := 0; i < numValidators; i++ {
 
 		nodeDirName := fmt.Sprintf("%s%d", viper.GetString(nodeDirPrefix), i)
 		nodeDir := filepath.Join(outDir, nodeDirName, "gaiad")
@@ -165,6 +144,36 @@ func testnetWithConfig(config *cfg.Config, cdc *wire.Codec, appInit AppInit) err
 	return nil
 }
 
+func getIP(i int) (ip string, err error) {
+	ip = viper.GetString(startingIPAddress)
+	if len(ip) == 0 {
+		ip, err = externalIP()
+		if err != nil {
+			return "", err
+		}
+	} else {
+		ip, err = calculateIP(ip, i)
+		if err != nil {
+			return "", err
+		}
+	}
+	return ip, nil
+}
+
+func writeFile(name string, dir string, contents []byte) error {
+	writePath := filepath.Join(dir)
+	file := filepath.Join(writePath, name)
+	err := cmn.EnsureDir(writePath, 0700)
+	if err != nil {
+		return err
+	}
+	err = cmn.WriteFile(file, contents, 0600)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func calculateIP(ip string, i int) (string, error) {
 	ipv4 := net.ParseIP(ip).To4()
 	if ipv4 == nil {

From 17b5370c2216f3a08acdc818843aefafa122a69a Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 16:08:35 -0700
Subject: [PATCH 08/11] Continue fixing gocyclo errors

---
 types/errors.go             |  1 +
 types/rational.go           | 47 +++++++++++++++++++++----------------
 x/stake/client/cli/tx.go    |  2 ++
 x/stake/client/rest/tx.go   |  2 ++
 x/stake/keeper/validator.go |  2 ++
 5 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/types/errors.go b/types/errors.go
index 6969549d900f..a106ee9bb7cf 100644
--- a/types/errors.go
+++ b/types/errors.go
@@ -66,6 +66,7 @@ const (
 )
 
 // NOTE: Don't stringer this, we'll put better messages in later.
+// nolint: gocyclo
 func CodeToDefaultMsg(code CodeType) string {
 	switch code {
 	case CodeInternal:
diff --git a/types/rational.go b/types/rational.go
index 24072fc09e81..f81ee083649e 100644
--- a/types/rational.go
+++ b/types/rational.go
@@ -37,6 +37,30 @@ func NewRat(Numerator int64, Denominator ...int64) Rat {
 	}
 }
 
+func getNumeratorDenominator(str []string, prec int) (numerator string, denom int64, err Error) {
+	switch len(str) {
+	case 1:
+		if len(str[0]) == 0 {
+			return "", 0, ErrUnknownRequest("not a decimal string")
+		}
+		numerator = str[0]
+		return numerator, 1, nil
+	case 2:
+		if len(str[0]) == 0 || len(str[1]) == 0 {
+			return "", 0, ErrUnknownRequest("not a decimal string")
+		}
+		if len(str[1]) > prec {
+			return "", 0, ErrUnknownRequest("string has too many decimals")
+		}
+		numerator = str[0] + str[1]
+		len := int64(len(str[1]))
+		denom = new(big.Int).Exp(big.NewInt(10), big.NewInt(len), nil).Int64()
+		return numerator, denom, nil
+	default:
+		return "", 0, ErrUnknownRequest("not a decimal string")
+	}
+}
+
 // create a rational from decimal string or integer string
 // precision is the number of values after the decimal point which should be read
 func NewRatFromDecimal(decimalStr string, prec int) (f Rat, err Error) {
@@ -53,26 +77,9 @@ func NewRatFromDecimal(decimalStr string, prec int) (f Rat, err Error) {
 
 	str := strings.Split(decimalStr, ".")
 
-	var numStr string
-	var denom int64 = 1
-	switch len(str) {
-	case 1:
-		if len(str[0]) == 0 {
-			return f, ErrUnknownRequest("not a decimal string")
-		}
-		numStr = str[0]
-	case 2:
-		if len(str[0]) == 0 || len(str[1]) == 0 {
-			return f, ErrUnknownRequest("not a decimal string")
-		}
-		if len(str[1]) > prec {
-			return f, ErrUnknownRequest("string has too many decimals")
-		}
-		numStr = str[0] + str[1]
-		len := int64(len(str[1]))
-		denom = new(big.Int).Exp(big.NewInt(10), big.NewInt(len), nil).Int64()
-	default:
-		return f, ErrUnknownRequest("not a decimal string")
+	numStr, denom, err := getNumeratorDenominator(str, prec)
+	if err != nil {
+		return f, err
 	}
 
 	num, errConv := strconv.Atoi(numStr)
diff --git a/x/stake/client/cli/tx.go b/x/stake/client/cli/tx.go
index 9b8acb8cc804..4701e4c20137 100644
--- a/x/stake/client/cli/tx.go
+++ b/x/stake/client/cli/tx.go
@@ -208,6 +208,8 @@ func GetCmdBeginRedelegate(storeName string, cdc *wire.Codec) *cobra.Command {
 	return cmd
 }
 
+// nolint: gocyclo
+// TODO: Make this pass gocyclo linting
 func getShares(storeName string, cdc *wire.Codec, sharesAmountStr, sharesPercentStr string,
 	delegatorAddr, validatorAddr sdk.Address) (sharesAmount sdk.Rat, err error) {
 
diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go
index 51a854528d96..4475d8c480c9 100644
--- a/x/stake/client/rest/tx.go
+++ b/x/stake/client/rest/tx.go
@@ -65,6 +65,8 @@ type EditDelegationsBody struct {
 	CompleteRedelegates []msgCompleteRedelegateInput `json:"complete_redelegates"`
 }
 
+// nolint: gocyclo
+// TODO: Split this up into several smaller functions, and remove the above nolint
 func editDelegationsRequestHandlerFn(cdc *wire.Codec, kb keys.Keybase, ctx context.CoreContext) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		var m EditDelegationsBody
diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go
index e8713c04d54c..69ca032f4e61 100644
--- a/x/stake/keeper/validator.go
+++ b/x/stake/keeper/validator.go
@@ -194,6 +194,8 @@ func (k Keeper) ClearTendermintUpdates(ctx sdk.Context) {
 // perfom all the nessisary steps for when a validator changes its power
 // updates all validator stores as well as tendermint update store
 // may kick out validators if new validator is entering the bonded validator group
+// nolint: gocyclo
+// TODO: Remove above nolint, function needs to be simplified
 func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator {
 	store := ctx.KVStore(k.storeKey)
 	pool := k.GetPool(ctx)

From 99e91dd2764587506ea1ebbdfb4de1abac0dd845 Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 16:16:43 -0700
Subject: [PATCH 09/11] Add nolints on remaining functions

---
 baseapp/baseapp.go          | 32 ++++++++++++++++++++------------
 client/keys/add.go          |  2 ++
 x/gov/client/rest/rest.go   |  2 ++
 x/stake/keeper/validator.go |  2 ++
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 0316663d35af..6375c5fc88ff 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -505,6 +505,23 @@ func (app *BaseApp) Deliver(tx sdk.Tx) (result sdk.Result) {
 	return app.runTx(runTxModeDeliver, nil, tx)
 }
 
+func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error {
+	if msgs == nil || len(msgs) == 0 {
+		// TODO: probably shouldn't be ErrInternal. Maybe new ErrInvalidMessage, or ?
+		return sdk.ErrInternal("Tx.GetMsgs() must return at least one message in list")
+	}
+
+	for _, msg := range msgs {
+		// Validate the Msg.
+		err := msg.ValidateBasic()
+		if err != nil {
+			err = err.WithDefaultCodespace(sdk.CodespaceRoot)
+			return err
+		}
+	}
+	return nil
+}
+
 // txBytes may be nil in some cases, eg. in tests.
 // Also, in the future we may support "internal" transactions.
 func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) {
@@ -533,18 +550,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 
 	// Get the Msg.
 	var msgs = tx.GetMsgs()
-	if msgs == nil || len(msgs) == 0 {
-		// TODO: probably shouldn't be ErrInternal. Maybe new ErrInvalidMessage, or ?
-		return sdk.ErrInternal("Tx.GetMsgs() must return at least one message in list").Result()
-	}
-
-	for _, msg := range msgs {
-		// Validate the Msg.
-		err := msg.ValidateBasic()
-		if err != nil {
-			err = err.WithDefaultCodespace(sdk.CodespaceRoot)
-			return err.Result()
-		}
+	err := validateBasicTxMsgs(msgs)
+	if err != nil {
+		return err.Result()
 	}
 
 	// Run the ante handler.
diff --git a/client/keys/add.go b/client/keys/add.go
index b7635468507f..22354ac0533c 100644
--- a/client/keys/add.go
+++ b/client/keys/add.go
@@ -46,6 +46,8 @@ phrase, otherwise, a new key will be generated.`,
 	return cmd
 }
 
+// nolint: gocyclo
+// TODO remove the above when addressing #1446
 func runAddCmd(cmd *cobra.Command, args []string) error {
 	var kb keys.Keybase
 	var err error
diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go
index 3e185dc914c0..1972b3dee297 100644
--- a/x/gov/client/rest/rest.go
+++ b/x/gov/client/rest/rest.go
@@ -369,6 +369,8 @@ func queryVoteHandlerFn(cdc *wire.Codec) http.HandlerFunc {
 	}
 }
 
+// nolint: gocyclo
+// todo: Split this functionality into helper functions to remove the above
 func queryProposalsWithParameterFn(cdc *wire.Codec) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		bechVoterAddr := r.URL.Query().Get(RestVoter)
diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go
index 69ca032f4e61..e19c2cd71d6b 100644
--- a/x/stake/keeper/validator.go
+++ b/x/stake/keeper/validator.go
@@ -286,6 +286,8 @@ func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) type
 // GetValidators.
 //
 // Optionally also return the validator from a retrieve address if the validator has been bonded
+// nolint: gocyclo
+// TODO: Remove the above golint
 func (k Keeper) UpdateBondedValidators(ctx sdk.Context,
 	affectedValidator types.Validator) (updatedVal types.Validator) {
 

From 472e168a72869a8b220f57e782768830319e14da Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 16:24:20 -0700
Subject: [PATCH 10/11] Minor refactor to reduce complexity, add nolint

---
 baseapp/baseapp.go | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 6375c5fc88ff..180bd20b2b11 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -522,8 +522,18 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error {
 	return nil
 }
 
+// Returns deliverState if app is in runTxModeDeliver, otherwhise returns checkstate
+func getState(app *BaseApp, mode runTxMode) *state {
+	if mode == runTxModeCheck || mode == runTxModeSimulate {
+		return app.checkState
+	} else {
+		return app.deliverState
+	}
+}
+
 // txBytes may be nil in some cases, eg. in tests.
 // Also, in the future we may support "internal" transactions.
+// nolint: gocyclo
 func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk.Result) {
 	//NOTE: GasWanted should be returned by the AnteHandler.
 	// GasUsed is determined by the GasMeter.
@@ -567,17 +577,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 		gasWanted = anteResult.GasWanted
 	}
 
-	// Get the correct cache
-	var msCache sdk.CacheMultiStore
-	if mode == runTxModeCheck || mode == runTxModeSimulate {
-		// CacheWrap app.checkState.ms in case it fails.
-		msCache = app.checkState.CacheMultiStore()
-		ctx = ctx.WithMultiStore(msCache)
-	} else {
-		// CacheWrap app.deliverState.ms in case it fails.
-		msCache = app.deliverState.CacheMultiStore()
-		ctx = ctx.WithMultiStore(msCache)
-	}
+	// Get the correct cache, CacheWrap app.checkState.ms in case it fails.
+	msCache := getState(app, mode).CacheMultiStore()
+	ctx = ctx.WithMultiStore(msCache)
 
 	// accumulate results
 	logs := make([]string, 0, len(msgs))

From 30c4abb39427850646dc202b20c46c9f12472aff Mon Sep 17 00:00:00 2001
From: ValarDragon <dojha12@gmail.com>
Date: Mon, 9 Jul 2018 16:27:51 -0700
Subject: [PATCH 11/11] Fix golint error

---
 baseapp/baseapp.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
index 180bd20b2b11..8a896286ee45 100644
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@@ -526,9 +526,8 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error {
 func getState(app *BaseApp, mode runTxMode) *state {
 	if mode == runTxModeCheck || mode == runTxModeSimulate {
 		return app.checkState
-	} else {
-		return app.deliverState
 	}
+	return app.deliverState
 }
 
 // txBytes may be nil in some cases, eg. in tests.