From c01e67db5959535c8ac03742e0ac1a73177d91c1 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 22 Feb 2023 16:21:34 -0300 Subject: [PATCH 1/6] fix: Format decimals correctly --- x/tx/textual/dec.go | 27 +++++++- x/tx/textual/dec_test.go | 35 +++++++++- x/tx/textual/int_test.go | 6 +- x/tx/textual/internal/testdata/decimals.json | 68 +++++++++++++++++--- 4 files changed, 123 insertions(+), 13 deletions(-) diff --git a/x/tx/textual/dec.go b/x/tx/textual/dec.go index 7639b60838a3..48b92548e92e 100644 --- a/x/tx/textual/dec.go +++ b/x/tx/textual/dec.go @@ -21,8 +21,33 @@ type decValueRenderer struct{} var _ ValueRenderer = decValueRenderer{} func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) { - formatted, err := math.FormatDec(v.String()) + decStr := v.String() + + // If the decimal doesn't contain a point, we assume it's a badly formatted + // value. So we assume it's a decimal with 18 decimal places and add the decimal + // in the right place. + if !strings.Contains(decStr, ".") { + isNeg := false + if strings.HasPrefix(decStr, "-") { + decStr = strings.TrimPrefix(decStr, "-") + isNeg = true + } + + if len(decStr) < 19 { + decStr = "0." + strings.Repeat("0", 18-len(decStr)) + decStr + } else { + whole, decimal := decStr[:len(decStr)-18], decStr[18:] + decStr = whole + "." + decimal + } + + if isNeg { + decStr = "-" + decStr + } + } + + formatted, err := math.FormatDec(decStr) if err != nil { + fmt.Println(decStr) return nil, err } return []Screen{{Content: formatted}}, nil diff --git a/x/tx/textual/dec_test.go b/x/tx/textual/dec_test.go index 1bf36575203d..3f76c71c3250 100644 --- a/x/tx/textual/dec_test.go +++ b/x/tx/textual/dec_test.go @@ -1,13 +1,17 @@ package textual_test import ( + "context" "encoding/json" + "math/big" "os" + "strings" "testing" "github.com/stretchr/testify/require" "google.golang.org/protobuf/reflect/protoreflect" + "cosmossdk.io/math" "cosmossdk.io/x/tx/textual" ) @@ -27,7 +31,36 @@ func TestDecJsonTestcases(t *testing.T) { r, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("SDKDEC")) require.NoError(t, err) - checkNumberTest(t, r, protoreflect.ValueOf(tc[0]), tc[1]) + checkDecTest(t, r, protoreflect.ValueOf(tc[0]), tc[1]) }) } } + +func checkDecTest(t *testing.T, r textual.ValueRenderer, pv protoreflect.Value, expected string) { + screens, err := r.Format(context.Background(), pv) + require.NoError(t, err) + require.Len(t, screens, 1) + require.Zero(t, screens[0].Indent) + require.False(t, screens[0].Expert) + + require.Equal(t, expected, screens[0].Content) + + // Round trip. + value, err := r.Parse(context.Background(), screens) + require.NoError(t, err) + + v1, err := math.LegacyNewDecFromStr(value.String()) + require.NoError(t, err) + + decStr := pv.String() + if !strings.Contains(decStr, ".") { + n, ok := new(big.Int).SetString(decStr, 10) + require.True(t, ok) + decStr = math.LegacyNewDecFromBigIntWithPrec(n, 18).String() + } + + v, err := math.LegacyNewDecFromStr(decStr) + require.NoError(t, err) + + require.Truef(t, v.Equal(v1), "%s != %s", v, v1) +} diff --git a/x/tx/textual/int_test.go b/x/tx/textual/int_test.go index 7b305221d71e..5d9680b8c97d 100644 --- a/x/tx/textual/int_test.go +++ b/x/tx/textual/int_test.go @@ -63,8 +63,8 @@ func checkNumberTest(t *testing.T, r textual.ValueRenderer, pv protoreflect.Valu screens, err := r.Format(context.Background(), pv) require.NoError(t, err) require.Len(t, screens, 1) - require.Equal(t, 0, screens[0].Indent) - require.Equal(t, false, screens[0].Expert) + require.Zero(t, screens[0].Indent) + require.False(t, screens[0].Expert) require.Equal(t, expected, screens[0].Content) @@ -78,5 +78,5 @@ func checkNumberTest(t *testing.T, r textual.ValueRenderer, pv protoreflect.Valu v1, err := math.LegacyNewDecFromStr(value.String()) require.NoError(t, err) - require.True(t, v.Equal(v1)) + require.Truef(t, v.Equal(v1), "%s != %s", v, v1) } diff --git a/x/tx/textual/internal/testdata/decimals.json b/x/tx/textual/internal/testdata/decimals.json index 3564b597d92b..e3685316e78a 100644 --- a/x/tx/textual/internal/testdata/decimals.json +++ b/x/tx/textual/internal/testdata/decimals.json @@ -1,9 +1,9 @@ [ - ["0", "0"], - ["1", "1"], - ["12", "12"], - ["123", "123"], - ["1234", "1'234"], + ["0.0", "0"], + ["1.0", "1"], + ["12.0", "12"], + ["123.0", "123"], + ["1234.0", "1'234"], ["0.1", "0.1"], ["0.01", "0.01"], ["0.001", "0.001"], @@ -41,7 +41,59 @@ ["0.000000000000000010", "0.00000000000000001"], ["0.000000000000000001", "0.000000000000000001"], ["-10.0", "-10"], - ["-10000", "-10'000"], - ["-9999", "-9'999"], - ["-999999999999", "-999'999'999'999"] + ["-10000.0", "-10'000"], + ["-9999.0", "-9'999"], + ["-999999999999.0", "-999'999'999'999"], + ["1000000000000000000000000", "1'000'000"], + ["1000000000000000000000000", "1'000'000"], + ["100000000000000000000000", "100'000"], + ["10000000000000000000000", "10'000"], + ["1000000000000000000000", "1'000"], + ["100000000000000000000", "100"], + ["10000000000000000000", "10"], + ["1000000000000000000", "1"], + ["100000000000000000", "0.1"], + ["10000000000000000", "0.01"], + ["1000000000000000", "0.001"], + ["100000000000000", "0.0001"], + ["10000000000000", "0.00001"], + ["1000000000000", "0.000001"], + ["100000000000", "0.0000001"], + ["10000000000", "0.00000001"], + ["1000000000", "0.000000001"], + ["100000000", "0.0000000001"], + ["10000000", "0.00000000001"], + ["1000000", "0.000000000001"], + ["100000", "0.0000000000001"], + ["10000", "0.00000000000001"], + ["1000", "0.000000000000001"], + ["100", "0.0000000000000001"], + ["10", "0.00000000000000001"], + ["1", "0.000000000000000001"], + ["-1000000000000000000000000", "-1'000'000"], + ["-1000000000000000000000000", "-1'000'000"], + ["-100000000000000000000000", "-100'000"], + ["-10000000000000000000000", "-10'000"], + ["-1000000000000000000000", "-1'000"], + ["-100000000000000000000", "-100"], + ["-10000000000000000000", "-10"], + ["-1000000000000000000", "-1"], + ["-100000000000000000", "-0.1"], + ["-10000000000000000", "-0.01"], + ["-1000000000000000", "-0.001"], + ["-100000000000000", "-0.0001"], + ["-10000000000000", "-0.00001"], + ["-1000000000000", "-0.000001"], + ["-100000000000", "-0.0000001"], + ["-10000000000", "-0.00000001"], + ["-1000000000", "-0.000000001"], + ["-100000000", "-0.0000000001"], + ["-10000000", "-0.00000000001"], + ["-1000000", "-0.000000000001"], + ["-100000", "-0.0000000000001"], + ["-10000", "-0.00000000000001"], + ["-1000", "-0.000000000000001"], + ["-100", "-0.0000000000000001"], + ["-10", "-0.00000000000000001"], + ["-1", "-0.000000000000000001"] ] From 21b12cf3d96505d598dc5b8d61e01208956ae207 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Wed, 22 Feb 2023 16:40:06 -0300 Subject: [PATCH 2/6] remove test print --- x/tx/textual/dec.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/tx/textual/dec.go b/x/tx/textual/dec.go index 48b92548e92e..de2c49ec96da 100644 --- a/x/tx/textual/dec.go +++ b/x/tx/textual/dec.go @@ -47,7 +47,6 @@ func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Sc formatted, err := math.FormatDec(decStr) if err != nil { - fmt.Println(decStr) return nil, err } return []Screen{{Content: formatted}}, nil From 619246fd90b238d469daa8aa5d5c71d3c73dca25 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 23 Feb 2023 12:36:20 -0300 Subject: [PATCH 3/6] use math instead of string --- x/tx/textual/dec.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/x/tx/textual/dec.go b/x/tx/textual/dec.go index de2c49ec96da..b4e205e34deb 100644 --- a/x/tx/textual/dec.go +++ b/x/tx/textual/dec.go @@ -3,6 +3,7 @@ package textual import ( "context" "fmt" + "math/big" "strings" "google.golang.org/protobuf/reflect/protoreflect" @@ -24,25 +25,16 @@ func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Sc decStr := v.String() // If the decimal doesn't contain a point, we assume it's a badly formatted - // value. So we assume it's a decimal with 18 decimal places and add the decimal - // in the right place. + // value. So we try to parse it as an integer and then convert it to a + // decimal. if !strings.Contains(decStr, ".") { - isNeg := false - if strings.HasPrefix(decStr, "-") { - decStr = strings.TrimPrefix(decStr, "-") - isNeg = true + parsedInt, ok := new(big.Int).SetString(decStr, 10) + if !ok { + return nil, fmt.Errorf("invalid decimal: %s", decStr) } - if len(decStr) < 19 { - decStr = "0." + strings.Repeat("0", 18-len(decStr)) + decStr - } else { - whole, decimal := decStr[:len(decStr)-18], decStr[18:] - decStr = whole + "." + decimal - } - - if isNeg { - decStr = "-" + decStr - } + // We assume the decimal has 18 digits of precision. + decStr = math.LegacyNewDecFromBigIntWithPrec(parsedInt, math.LegacyPrecision).String() } formatted, err := math.FormatDec(decStr) From 507c3a16773f29e36b5b1f643ba32807faf99332 Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Thu, 23 Feb 2023 15:17:58 -0300 Subject: [PATCH 4/6] Update x/tx/textual/dec.go Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> --- x/tx/textual/dec.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/tx/textual/dec.go b/x/tx/textual/dec.go index b4e205e34deb..396d7ac6da95 100644 --- a/x/tx/textual/dec.go +++ b/x/tx/textual/dec.go @@ -24,8 +24,8 @@ var _ ValueRenderer = decValueRenderer{} func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) { decStr := v.String() - // If the decimal doesn't contain a point, we assume it's a badly formatted - // value. So we try to parse it as an integer and then convert it to a + // If the decimal doesn't contain a point, we assume it's a value formatted using the legacy + // `sdk.Dec`. So we try to parse it as an integer and then convert it to a // decimal. if !strings.Contains(decStr, ".") { parsedInt, ok := new(big.Int).SetString(decStr, 10) From ac6ebe3c81f5f9560c1c3cae72681f9968476eba Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 23 Feb 2023 15:35:10 -0300 Subject: [PATCH 5/6] fix lint --- types/mempool/sender_nonce.go | 6 +++--- x/gov/genesis.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index 7f41482d69ca..928d6319bdc1 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -37,7 +37,7 @@ type SenderNonceMempool struct { existingTx map[txKey]bool } -type SenderNonceOptions func(mp *SenderNonceMempool) +type SenderNonceOptions func(*SenderNonceMempool) type txKey struct { address string @@ -103,8 +103,8 @@ func (snm *SenderNonceMempool) setSeed(seed int64) { // NextSenderTx returns the next transaction for a given sender by nonce order, // i.e. the next valid transaction for the sender. If no such transaction exists, // nil will be returned. -func (mp *SenderNonceMempool) NextSenderTx(sender string) sdk.Tx { - senderIndex, ok := mp.senders[sender] +func (snm *SenderNonceMempool) NextSenderTx(sender string) sdk.Tx { + senderIndex, ok := snm.senders[sender] if !ok { return nil } diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 1b5b1646b0c1..c8bdf162049e 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -51,7 +51,6 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k if !balance.Equal(totalDeposits) { panic(fmt.Sprintf("expected module account was %s but we got %s", balance.String(), totalDeposits.String())) } - } // ExportGenesis - output genesis parameters From 57fcc1b89ba7ce1a1eec0a1a774f52190d44e539 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Thu, 23 Feb 2023 16:09:34 -0300 Subject: [PATCH 6/6] lint --- x/gov/keeper/constitution.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/keeper/constitution.go b/x/gov/keeper/constitution.go index 1e18aa9e196b..db3ab6e3913a 100644 --- a/x/gov/keeper/constitution.go +++ b/x/gov/keeper/constitution.go @@ -14,5 +14,5 @@ func (keeper Keeper) GetConstitution(ctx sdk.Context) (constitution string) { func (keeper Keeper) SetConstitution(ctx sdk.Context, constitution string) { store := ctx.KVStore(keeper.storeKey) - store.Set([]byte(types.KeyConstitution), []byte(constitution)) + store.Set(types.KeyConstitution, []byte(constitution)) }