From 59196278502434e7c62148ad47bf1a3307479c4d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 11 Feb 2019 11:16:20 -0500 Subject: [PATCH 1/6] use vanilla JSON encoder for ABCI log (to allow unicode) --- types/errors.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/types/errors.go b/types/errors.go index 9f728adfa895..743056e1c124 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 error for ABCI log")) } - stringifiedJSON := string(bz) - return stringifiedJSON + + return buff.String() } func (err *sdkError) Result() Result { From 95c4feb24f8b3db1ad072b65fdc10a928d999dc7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 11 Feb 2019 11:21:04 -0500 Subject: [PATCH 2/6] improve deduct fees error messages --- types/errors.go | 2 +- x/auth/ante.go | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/types/errors.go b/types/errors.go index 743056e1c124..99b855b920bc 100644 --- a/types/errors.go +++ b/types/errors.go @@ -259,7 +259,7 @@ func (err *sdkError) ABCILog() string { enc.SetEscapeHTML(false) if err := enc.Encode(jsonErr); err != nil { - panic(errors.Wrap(err, "failed to encode error for ABCI log")) + panic(errors.Wrap(err, "failed to encode ABCI error log")) } return buff.String() diff --git a/x/auth/ante.go b/x/auth/ante.go index ca09e3210a12..ddf9dbecd158 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{} From 67996217944c16099d85a799f5a4fc7d9e88a604 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 11 Feb 2019 11:24:05 -0500 Subject: [PATCH 3/6] improve insufficient funds error messages in x/bank keeper --- x/bank/keeper.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index bc4fa787c9ce..e7a93ce7148a 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 { From b9988303a3876cc0ef12703cc4b3d9aaf309cf01 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 11 Feb 2019 11:25:52 -0500 Subject: [PATCH 4/6] add pending log --- PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PENDING.md b/PENDING.md index bf8ca309a0d3..d35a0a9a59ec 100644 --- a/PENDING.md +++ b/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 From 9b512462aedc75f98a1dce293d66cac05ca31455 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 11 Feb 2019 11:44:54 -0500 Subject: [PATCH 5/6] trim space --- types/errors.go | 2 +- types/errors_test.go | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/types/errors.go b/types/errors.go index 99b855b920bc..82f400c54ff4 100644 --- a/types/errors.go +++ b/types/errors.go @@ -262,7 +262,7 @@ func (err *sdkError) ABCILog() string { panic(errors.Wrap(err, "failed to encode ABCI error log")) } - return buff.String() + return strings.TrimSpace(buff.String()) } func (err *sdkError) Result() Result { diff --git a/types/errors_test.go b/types/errors_test.go index 8d4f8822633e..52de6e14f6a5 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)) } From cb3643c63e4e17f81bc7b06efdf55dc30ce11688 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Mon, 11 Feb 2019 11:53:35 -0500 Subject: [PATCH 6/6] Update PENDING.md --- PENDING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PENDING.md b/PENDING.md index d35a0a9a59ec..6571be63b43e 100644 --- a/PENDING.md +++ b/PENDING.md @@ -34,7 +34,7 @@ IMPROVEMENTS * Gaia * SDK - * [\#3601] Improve SDK funds related error messages and allow for unicode in + * [\#3604] Improve SDK funds related error messages and allow for unicode in JSON ABCI log. * Tendermint @@ -52,4 +52,4 @@ BUG FIXES * SDK * [\#3582](https://github.com/cosmos/cosmos-sdk/pull/3582) Running `make test_unit was failing due to a missing tag -* Tendermint \ No newline at end of file +* Tendermint