R4R: Improve SDK Error Messages & Allow Unicode

PENDING.md

@@ -34,6 +34,8 @@ IMPROVEMENTS
 * Gaia
 
 * SDK
+  * [\#3601] Improve SDK funds related error messages and allow for unicode in
+  JSON ABCI log.
 
 * Tendermint
 

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 buff.String()
 }
 
 func (err *sdkError) Result() Result {

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{}

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 {