-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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.
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
refactor!: create math go sub module #11788
Conversation
@aaronc we need to duplicate // ToDec converts Int to Dec
func (i Int) ToDec() Dec {
return NewDecFromInt(i)
} Should we:
(1) allows use to avoid copying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we alias the math package from types for a release and mark as deprecated, similar to errors
Yes :) |
I chatted with @alexanderbez about this a bit, but the idea is to keep the gogo proto and amino serialization in |
What I'd propose for So in type Int math.Int
func (i Int) ToDec() Dec {
return NewDecFromInt(i)
} I'd prefer to avoid API breakage in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'd propose is (for now at least) to leave all of this stuff in types/int.go
on the types.Int
wrapper type:
Lines 335 to 434 in 019444a
func (i Int) MarshalJSON() ([]byte, error) { | |
if i.i == nil { // Necessary since default Uint initialization has i.i as nil | |
i.i = new(big.Int) | |
} | |
return marshalJSON(i.i) | |
} | |
// UnmarshalJSON defines custom decoding scheme | |
func (i *Int) UnmarshalJSON(bz []byte) error { | |
if i.i == nil { // Necessary since default Int initialization has i.i as nil | |
i.i = new(big.Int) | |
} | |
return unmarshalJSON(i.i, bz) | |
} | |
// MarshalJSON for custom encoding scheme | |
// Must be encoded as a string for JSON precision | |
func marshalJSON(i encoding.TextMarshaler) ([]byte, error) { | |
text, err := i.MarshalText() | |
if err != nil { | |
return nil, err | |
} | |
return json.Marshal(string(text)) | |
} | |
// UnmarshalJSON for custom decoding scheme | |
// Must be encoded as a string for JSON precision | |
func unmarshalJSON(i *big.Int, bz []byte) error { | |
var text string | |
if err := json.Unmarshal(bz, &text); err != nil { | |
return err | |
} | |
return unmarshalText(i, text) | |
} | |
// MarshalYAML returns the YAML representation. | |
func (i Int) MarshalYAML() (interface{}, error) { | |
return i.String(), nil | |
} | |
// Marshal implements the gogo proto custom type interface. | |
func (i Int) Marshal() ([]byte, error) { | |
if i.i == nil { | |
i.i = new(big.Int) | |
} | |
return i.i.MarshalText() | |
} | |
// MarshalTo implements the gogo proto custom type interface. | |
func (i *Int) MarshalTo(data []byte) (n int, err error) { | |
if i.i == nil { | |
i.i = new(big.Int) | |
} | |
if i.i.BitLen() == 0 { // The value 0 | |
copy(data, []byte{0x30}) | |
return 1, nil | |
} | |
bz, err := i.Marshal() | |
if err != nil { | |
return 0, err | |
} | |
copy(data, bz) | |
return len(bz), nil | |
} | |
// Unmarshal implements the gogo proto custom type interface. | |
func (i *Int) Unmarshal(data []byte) error { | |
if len(data) == 0 { | |
i = nil | |
return nil | |
} | |
if i.i == nil { | |
i.i = new(big.Int) | |
} | |
if err := i.i.UnmarshalText(data); err != nil { | |
return err | |
} | |
if i.i.BitLen() > maxBitLen { | |
return fmt.Errorf("integer out of range; got: %d, max: %d", i.i.BitLen(), maxBitLen) | |
} | |
return nil | |
} | |
// Size implements the gogo proto custom type interface. | |
func (i *Int) Size() int { | |
bz, _ := i.Marshal() | |
return len(bz) | |
} | |
// Override Amino binary serialization by proxying to protobuf. | |
func (i Int) MarshalAmino() ([]byte, error) { return i.Marshal() } | |
func (i *Int) UnmarshalAmino(bz []byte) error { return i.Unmarshal(bz) } |
To avoid carrying all the gogo/amino baggage
… into bez/11784-math-sub-mod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you decide to not create an Int wrapper type with the Dec method defined?
Otherwise LGTM
Yes, correct. That one API was just not worth it -- a caller can and should just call |
Description
Closes: #11784
math
go sub-moduleInt
, andUint
types tomath
types/math.go
Int#ToDec
(API breaking)A beta release/tag of
math
will be made along with thereplace
directive being removed once this PR is merged.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change