Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Implement custom hybrid un-/marshal model #22529

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+164 −47
Open

feat: Implement custom hybrid un-/marshal model #22529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 79 additions & 4 deletions math/dec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
stderrors "errors"
"math/big"
"strconv"

"github.com/cockroachdb/apd/v3"

Expand Down Expand Up @@ -407,14 +408,88 @@ func (x Dec) Reduce() (Dec, int) {
return y, n
}

// Marshal serializes the decimal value into a byte slice in text format.
// This method represents the decimal in a portable and compact hybrid notation.
// Based on the exponent value, the number is formatted into decimal: -ddddd.ddddd, no exponent
// or scientific notation: -d.ddddE±dd
//
// For example, the following transformations are made:
// - 0 -> 0
// - 123 -> 123
// - 10000 -> 10000
// - -0.001 -> -0.001
// - -0.000000001 -> -1E-9
//
// Returns:
// - A byte slice of the decimal in text format.
// - An error if the decimal cannot be reduced or marshaled properly.
func (x Dec) Marshal() ([]byte, error) {
// implemented in a new PR. See: https://github.com/cosmos/cosmos-sdk/issues/22525
panic("not implemented")
var d apd.Decimal
if _, _, err := dec128Context.Reduce(&d, &x.dec); err != nil {
return nil, ErrInvalidDec.Wrap(err.Error())
}
return fmtE(d, 'E'), nil
}

// custom formatter
func fmtE(d apd.Decimal, fmt byte) []byte {
var scratch, dest [16]byte
buf := dest[:0]
digits := d.Coeff.Append(scratch[:0], 10)
totalDigits := int64(len(digits))
adj := int64(d.Exponent) + totalDigits - 1
if adj > -6 && adj < 6 {
return []byte(d.Text('f'))
}
switch {
case totalDigits > 5:
beforeComma := digits[0 : totalDigits-6]
adj -= int64(len(beforeComma) - 1)
buf = append(buf, beforeComma...)
buf = append(buf, '.')
buf = append(buf, digits[totalDigits-6:]...)
case totalDigits > 1:
buf = append(buf, digits[0])
buf = append(buf, '.')
buf = append(buf, digits[1:]...)
default:
buf = append(buf, digits[0:]...)
}

buf = append(buf, fmt)
var ch byte
if adj < 0 {
ch = '-'
adj = -adj
} else {
ch = '+'
}
buf = append(buf, ch)
return strconv.AppendInt(buf, adj, 10)
}

// isEmptyExp checks if the adjusted exponent of the given decimal is zero.
func isEmptyExp(d apd.Decimal) bool {
var scratch [16]byte
digits := d.Coeff.Append(scratch[:0], 10)
adj := int64(d.Exponent) + int64(len(digits)) - 1
return adj == 0
}

// Unmarshal parses a byte slice containing a text-formatted decimal and stores the result in the receiver.
// It returns an error if the byte slice does not represent a valid decimal.
func (x *Dec) Unmarshal(data []byte) error {
// implemented in a new PR. See: https://github.com/cosmos/cosmos-sdk/issues/22525
panic("not implemented")
result, err := NewDecFromString(string(data))
if err != nil {
return ErrInvalidDec.Wrap(err.Error())
}

if result.dec.Form != apd.Finite {
return ErrInvalidDec.Wrap("unknown decimal form")
}

x.dec = result.dec
return nil
}

// MarshalTo encodes the receiver into the provided byte slice and returns the number of bytes written and any error encountered.
Expand Down
128 changes: 85 additions & 43 deletions math/dec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1309,55 +1309,38 @@ func must[T any](r T, err error) T {
}

func TestMarshalUnmarshal(t *testing.T) {
t.Skip("not supported, yet")
specs := map[string]struct {
x Dec
exp string
expErr error
}{
"No trailing zeros": {
x: NewDecFromInt64(123456),
exp: "1.23456E+5",
},
"Trailing zeros": {
x: NewDecFromInt64(123456000),
exp: "1.23456E+8",
},
"Zero value": {
x: NewDecFromInt64(0),
exp: "0E+0",
exp: "0",
},
"-0": {
x: NewDecFromInt64(-0),
exp: "0E+0",
},
"Decimal value": {
x: must(NewDecFromString("1.30000")),
exp: "1.3E+0",
},
"Positive value": {
x: NewDecFromInt64(10),
exp: "1E+1",
exp: "0",
},
"negative 10": {
x: NewDecFromInt64(-10),
exp: "-1E+1",
"1 decimal place": {
x: must(NewDecFromString("0.1")),
exp: "0.1",
},
"9 with trailing zeros": {
x: must(NewDecFromString("9." + strings.Repeat("0", 34))),
exp: "9E+0",
"2 decimal places": {
x: must(NewDecFromString("0.01")),
exp: "0.01",
},
"negative 1 with negative exponent zeros": {
x: must(NewDecFromString("-1.000001")),
exp: "-1.000001E+0",
"3 decimal places": {
x: must(NewDecFromString("0.001")),
exp: "0.001",
},
"negative 1 with trailing zeros": {
x: must(NewDecFromString("-1." + strings.Repeat("0", 34))),
exp: "-1E+0",
"4 decimal places": {
x: must(NewDecFromString("0.0001")),
exp: "0.0001",
},
"5 decimal places": {
x: must(NewDecFromString("0.00001")),
exp: "1E-5",
exp: "0.00001",
},
"6 decimal places": {
x: must(NewDecFromString("0.000001")),
Expand All @@ -1367,17 +1350,73 @@ func TestMarshalUnmarshal(t *testing.T) {
x: must(NewDecFromString("0.0000001")),
exp: "1E-7",
},
"1": {
x: must(NewDecFromString("1")),
exp: "1",
},
"12": {
x: must(NewDecFromString("12")),
exp: "12",
},
"123": {
x: must(NewDecFromString("123")),
exp: "123",
},
"1234": {
x: must(NewDecFromString("1234")),
exp: "1234",
},
"12345": {
x: must(NewDecFromString("12345")),
exp: "12345",
},
"123456": {
x: must(NewDecFromString("123456")),
exp: "123456",
},
"1234567": {
x: must(NewDecFromString("1234567")),
exp: "1.234567E+6",
},
"12345678": {
x: must(NewDecFromString("12345678")),
exp: "12.345678E+6",
},
"123456789": {
x: must(NewDecFromString("123456789")),
exp: "123.456789E+6",
},
"1234567890": {
x: must(NewDecFromString("1234567890")),
exp: "123.456789E+7",
},
"12345678900": {
x: must(NewDecFromString("12345678900")),
exp: "123.456789E+8",
},
Comment on lines +1353 to +1396
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add boundary test cases for scientific notation transition.

The test cases cover various integer values but could benefit from explicit boundary test cases at the scientific notation transition points.

Add these test cases:

"boundary/decimal_to_scientific_positive": {
    x:   must(NewDecFromString("999999")),  // Last number in decimal
    exp: "999999",
},
"boundary/decimal_to_scientific_positive_next": {
    x:   must(NewDecFromString("1000000")), // First number in scientific
    exp: "1E+6",
},
"boundary/decimal_to_scientific_negative": {
    x:   must(NewDecFromString("0.000001")), // Last number in decimal
    exp: "0.000001",
},
"boundary/decimal_to_scientific_negative_next": {
    x:   must(NewDecFromString("0.0000001")), // First number in scientific
    exp: "1E-7",
},

"negative 1 with negative exponent": {
x: must(NewDecFromString("-1.000001")),
exp: "-1.000001",
},
"-1.0000001 - negative 1 with negative exponent": {
x: must(NewDecFromString("-1.0000001")),
exp: "-1.0000001",
},
"3 decimal places before the comma": {
x: must(NewDecFromString("100")),
exp: "100",
},
"4 decimal places before the comma": {
x: must(NewDecFromString("1000")),
exp: "1E+3",
exp: "1000",
},
"5 decimal places before the comma": {
x: must(NewDecFromString("10000")),
exp: "1E+4",
exp: "10000",
},
"6 decimal places before the comma": {
x: must(NewDecFromString("100000")),
exp: "1E+5",
exp: "100000",
},
"7 decimal places before the comma": {
x: must(NewDecFromString("1000000")),
Expand All @@ -1388,12 +1427,12 @@ func TestMarshalUnmarshal(t *testing.T) {
exp: "1E+100000",
},
"1.1e100000": {
x: NewDecWithExp(11, 100_000),
expErr: ErrInvalidDec,
x: must(NewDecFromString("1.1e100000")),
exp: "1.1E+100000",
},
"1.e100000": {
x: NewDecWithExp(1, 100_000),
exp: "1E+100000",
"1e100001": {
x: NewDecWithExp(1, 100_001),
expErr: ErrInvalidDec,
},
}
for name, spec := range specs {
Expand All @@ -1404,9 +1443,12 @@ func TestMarshalUnmarshal(t *testing.T) {
return
}
require.NoError(t, gotErr)
unmarshalled := new(Dec)
require.NoError(t, unmarshalled.Unmarshal(marshaled))
assert.Equal(t, spec.exp, unmarshalled.dec.Text('E'))
assert.Equal(t, spec.exp, string(marshaled))
// and backwards
unmarshalledDec := new(Dec)
require.NoError(t, unmarshalledDec.Unmarshal(marshaled))
assert.Equal(t, spec.x.String(), unmarshalledDec.String())
assert.True(t, spec.x.Equal(*unmarshalledDec))
Comment on lines +1446 to +1451
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add unmarshal error test cases.

The unmarshaling tests only verify successful cases. Consider adding test cases for invalid input.

Add these test cases:

"unmarshal/invalid_format": {
    x:      NewDecFromInt64(0),
    exp:    "invalid",
    expErr: ErrInvalidDec,
},
"unmarshal/empty_input": {
    x:      NewDecFromInt64(0),
    exp:    "",
    expErr: ErrInvalidDec,
},

})
}
}
Loading