From 9c23fe68eee6d7c5acef749330bc30e9ff1f25e1 Mon Sep 17 00:00:00 2001
From: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Date: Mon, 11 Feb 2019 18:12:43 -0500
Subject: [PATCH] Merge PR #3604: Improve REST Error Messages & Allow Unicode

---
 PENDING.md           |  2 ++
 types/errors.go      | 20 ++++++++++++--------
 types/errors_test.go | 19 ++++++++++++-------
 x/auth/ante.go       | 12 +++++++-----
 x/bank/keeper.go     | 12 +++++++++---
 5 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/PENDING.md b/PENDING.md
index e8c6f7bdc6..4828030dc5 100644
--- a/PENDING.md
+++ b/PENDING.md
@@ -34,6 +34,8 @@ IMPROVEMENTS
 * Gaia
 
 * SDK
+  * [\#3604] Improve SDK funds related error messages and allow for unicode in
+  JSON ABCI log.
 
 * Tendermint
 
diff --git a/types/errors.go b/types/errors.go
index 9f728adfa8..82f400c54f 100644
--- a/types/errors.go
+++ b/types/errors.go
@@ -1,13 +1,14 @@
 package types
 
 import (
+	"bytes"
+	"encoding/json"
 	"fmt"
 	"strings"
 
+	"github.com/pkg/errors"
 	cmn "github.com/tendermint/tendermint/libs/common"
 
-	"github.com/cosmos/cosmos-sdk/codec"
-
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
@@ -246,19 +247,22 @@ func (err *sdkError) Code() CodeType {
 
 // Implements ABCIError.
 func (err *sdkError) ABCILog() string {
-	cdc := codec.New()
 	errMsg := err.cmnError.Error()
 	jsonErr := humanReadableError{
 		Codespace: err.codespace,
 		Code:      err.code,
 		Message:   errMsg,
 	}
-	bz, er := cdc.MarshalJSON(jsonErr)
-	if er != nil {
-		panic(er)
+
+	var buff bytes.Buffer
+	enc := json.NewEncoder(&buff)
+	enc.SetEscapeHTML(false)
+
+	if err := enc.Encode(jsonErr); err != nil {
+		panic(errors.Wrap(err, "failed to encode ABCI error log"))
 	}
-	stringifiedJSON := string(bz)
-	return stringifiedJSON
+
+	return strings.TrimSpace(buff.String())
 }
 
 func (err *sdkError) Result() Result {
diff --git a/types/errors_test.go b/types/errors_test.go
index 8d4f882263..52de6e14f6 100644
--- a/types/errors_test.go
+++ b/types/errors_test.go
@@ -72,18 +72,23 @@ func TestAppendMsgToErr(t *testing.T) {
 
 		// plain msg error
 		msg := AppendMsgToErr("something unexpected happened", errMsg)
-		require.Equal(t, fmt.Sprintf("something unexpected happened; %s",
-			errMsg),
+		require.Equal(
+			t,
+			fmt.Sprintf("something unexpected happened; %s", errMsg),
 			msg,
-			fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i))
+			fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i),
+		)
 
 		// ABCI Log msg error
 		msg = AppendMsgToErr("something unexpected happened", abciLog)
 		msgIdx := mustGetMsgIndex(abciLog)
-		require.Equal(t, fmt.Sprintf("%s%s; %s}",
-			abciLog[:msgIdx],
-			"something unexpected happened",
-			abciLog[msgIdx:len(abciLog)-1]),
+		require.Equal(
+			t,
+			fmt.Sprintf("%s%s; %s}",
+				abciLog[:msgIdx],
+				"something unexpected happened",
+				abciLog[msgIdx:len(abciLog)-1],
+			),
 			msg,
 			fmt.Sprintf("Should have formatted the error message of ABCI Log. tc #%d", i))
 	}
diff --git a/x/auth/ante.go b/x/auth/ante.go
index ca09e3210a..ddf9dbecd1 100644
--- a/x/auth/ante.go
+++ b/x/auth/ante.go
@@ -306,20 +306,22 @@ func DeductFees(blockTime time.Time, acc Account, fee StdFee) (Account, sdk.Resu
 	// get the resulting coins deducting the fees
 	newCoins, ok := coins.SafeMinus(feeAmount)
 	if ok {
-		errMsg := fmt.Sprintf("%s < %s", coins, feeAmount)
-		return nil, sdk.ErrInsufficientFunds(errMsg).Result()
+		return nil, sdk.ErrInsufficientFunds(
+			fmt.Sprintf("insufficient funds to pay for fees; %s < %s", coins, feeAmount),
+		).Result()
 	}
 
 	// Validate the account has enough "spendable" coins as this will cover cases
 	// such as vesting accounts.
 	spendableCoins := acc.SpendableCoins(blockTime)
 	if _, hasNeg := spendableCoins.SafeMinus(feeAmount); hasNeg {
-		return nil, sdk.ErrInsufficientFunds(fmt.Sprintf("%s < %s", spendableCoins, feeAmount)).Result()
+		return nil, sdk.ErrInsufficientFunds(
+			fmt.Sprintf("insufficient funds to pay for fees; %s < %s", spendableCoins, feeAmount),
+		).Result()
 	}
 
 	if err := acc.SetCoins(newCoins); err != nil {
-		// Handle w/ #870
-		panic(err)
+		return nil, sdk.ErrInternal(err.Error()).Result()
 	}
 
 	return acc, sdk.Result{}
diff --git a/x/bank/keeper.go b/x/bank/keeper.go
index bc4fa787c9..e7a93ce714 100644
--- a/x/bank/keeper.go
+++ b/x/bank/keeper.go
@@ -230,7 +230,9 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress,
 	// So the check here is sufficient instead of subtracting from oldCoins.
 	_, hasNeg := spendableCoins.SafeMinus(amt)
 	if hasNeg {
-		return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", spendableCoins, amt))
+		return amt, nil, sdk.ErrInsufficientCoins(
+			fmt.Sprintf("insufficient account funds; %s < %s", spendableCoins, amt),
+		)
 	}
 
 	newCoins := oldCoins.Minus(amt) // should not panic as spendable coins was already checked
@@ -246,7 +248,9 @@ func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s
 	newCoins := oldCoins.Plus(amt)
 
 	if newCoins.IsAnyNegative() {
-		return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt))
+		return amt, nil, sdk.ErrInsufficientCoins(
+			fmt.Sprintf("insufficient account funds; %s < %s", oldCoins, amt),
+		)
 	}
 
 	err := setCoins(ctx, am, addr, newCoins)
@@ -319,7 +323,9 @@ func delegateCoins(
 
 	_, hasNeg := oldCoins.SafeMinus(amt)
 	if hasNeg {
-		return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt))
+		return nil, sdk.ErrInsufficientCoins(
+			fmt.Sprintf("insufficient account funds; %s < %s", oldCoins, amt),
+		)
 	}
 
 	if err := trackDelegation(acc, ctx.BlockHeader().Time, amt); err != nil {