`sdk.Int` marshaling of empty instance is always 0 instead of `"<nil>"`

[fedekunze]

Summary of Bug

Marshaling of an empty sdk.Int always populates the big.Int value to zero. This is unexpected from applications as the encoding of a nil value for big.Int should be "<nil>".

// MarshalText implements the encoding.TextMarshaler interface.
func (x *Int) MarshalText() (text []byte, err error) {
	if x == nil {
		return []byte("<nil>"), nil
	}
	return x.abs.itoa(x.neg, 10), nil
}

Version

v0.43.0-rc0

Steps to Reproduce

• create an empty sdk.Int instance and call Marshal

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[fedekunze]

I also see that UnmarshalText fails when the nil string is provided. What should be the expected behavior here? As it's not documented in the sdk.Int comments, I don't know what to expect after providing a nil *big.Int

[fedekunze]

cc: @odeke-em @alexanderbez @aaronc @zmanian @clevinson

[fedekunze]

I also don't understand why we need to both marshal json and marshal text:

EDIT: I see now that it's to display the json value for sdk.Int as string instead of a number

cosmos-sdk/types/int.go

Lines 329 to 365 in 1b0b8e9

	// MarshalJSON defines custom encoding scheme
	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)
	}

[alexanderbez]

I cannot recall why we explicitly have both text and JSON marshalling variants. We could probably remove the text variant and just copy the logic directly into the JSON variant.

As for <"nil"> vs "", we should stick with the idiomatic "<nil>" probably?

[spoo-bar]

The way big.Int handles encoding, a nil value is encoded to a [48]. And as sdk.Int is a wrapper around big.Int, I feel sdk.Int{} encoded value to be zero is expected behavior.

Additionally, with ref to MarshalJSON method, I am not sure a JSON value which is expected to be an number should have a string "<nil>" value. IF nil values should be handled explicitly, I think using null in JSON would be a better option?

cosmos-sdk/types/int.go

Lines 329 to 335 in 1b0b8e9

	// MarshalJSON defines custom encoding scheme
	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)
	}

[alexanderbez]

We should use whatever the standard encoded JSON value is for zero-valued types, which is "<nil>", no?

[spoo-bar]

Ref 1 - This doc says null is to be used in JSON when no value is assigned.
Ref 2 - This doc recommends ignoring null values in JSON object altogether.

Afaik, <nil> isn't usually used in JSON objects.

[alexanderbez]

Yes, sorry "null". As for whether or not it's included, it depends on the spec and context. We can choose to omit and return nil in MarshalJSON.

[robert-zaremba]

This is a big breaking change and breaks tonnes of tests. In my opinion we should follow the number (eg int) behavior. big.Int doesn't have a nil value. *big.Int has. We are creating our own Int type. Do we want to think about it as int or *int? If the former one, then it makes sense to serialize it as zero, unless json:"omitempty" is provided (then we skip the whole attribute).

BTW: do we use in any place MarshalText?

[alexanderbez]

It should really be treated as int and not *int. Clients would call isZero as an analogous == nil check.