diff --git a/CHANGELOG.md b/CHANGELOG.md index 787216cbd6..e1bdbfd9f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ This patch update also includes minor dependency bumps. ### Features * (abci_utils) [#25008](https://github.com/cosmos/cosmos-sdk/pull/24861) add the ability to assign a custom signer extraction adapter in `DefaultProposalHandler`. +* (x/tx) [#25539](https://github.com/cosmos/cosmos-sdk/pull/25539) Expose `NullSliceAsEmptyEncoder` as a public function and add `null_slice_as_empty` encoding option for protobuf annotations. ## [v0.53.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.53.3) - 2025-07-08 diff --git a/docs/docs/build/building-modules/05-protobuf-annotations.md b/docs/docs/build/building-modules/05-protobuf-annotations.md index 942b9a892f..8757158251 100644 --- a/docs/docs/build/building-modules/05-protobuf-annotations.md +++ b/docs/docs/build/building-modules/05-protobuf-annotations.md @@ -122,8 +122,19 @@ https://github.com/cosmos/cosmos-sdk/blob/e8f28bf5db18b8d6b7e0d94b542ce4cf48fed9 Encoding instructs the amino json marshaler how to encode certain fields that may differ from the standard encoding behaviour. The most common example of this is how `repeated cosmos.base.v1beta1.Coin` is encoded when using the amino json encoding format. The `legacy_coins` option tells the json marshaler [how to encode a null slice](https://github.com/cosmos/cosmos-sdk/blob/e8f28bf5db18b8d6b7e0d94b542ce4cf48fed9d6/x/tx/signing/aminojson/json_marshal.go#L65) of `cosmos.base.v1beta1.Coin`. +For a more generic option that works with any slice type, you can use `null_slice_as_empty`, which ensures that nil slices are encoded as empty arrays (`[]`) instead of `null`. This is useful for maintaining backward compatibility with legacy Amino JSON encoding where nil slices were serialized as empty arrays. + +Alternatively, you can use the exported `NullSliceAsEmptyEncoder` function directly in your code: + +```go +encoder := aminojson.NewEncoder(options) +encoder = encoder.DefineFieldEncoding("my_field", aminojson.NullSliceAsEmptyEncoder) +``` + ```proto (amino.encoding) = "legacy_coins", +// or for a more generic option: +(amino.encoding) = "null_slice_as_empty", ``` ```proto reference diff --git a/x/tx/signing/aminojson/aminojson_test.go b/x/tx/signing/aminojson/aminojson_test.go index 183b66a1d3..2abd7cea6a 100644 --- a/x/tx/signing/aminojson/aminojson_test.go +++ b/x/tx/signing/aminojson/aminojson_test.go @@ -234,3 +234,90 @@ func TestNewSignModeHandler(t *testing.T) { }) require.NotNil(t, handler) } + +func TestNullSliceAsEmptyEncoder(t *testing.T) { + encoder := aminojson.NewEncoder(aminojson.EncoderOptions{}) + + testCases := []struct { + name string + amount []*basev1beta1.Coin + wantJSON string + description string + }{ + { + name: "empty slice encodes as empty array", + amount: []*basev1beta1.Coin{}, + wantJSON: `[]`, + description: "Empty slice should be encoded as [] not null", + }, + { + name: "nil slice encodes as empty array", + amount: nil, + wantJSON: `[]`, + description: "Nil slice should be encoded as [] not null", + }, + { + name: "non-empty slice encodes normally", + amount: []*basev1beta1.Coin{ + {Denom: "uatom", Amount: "1000"}, + {Denom: "stake", Amount: "500"}, + }, + wantJSON: "", // Will check content instead of exact match + description: "Non-empty slice should encode normally", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fee := &txv1beta1.Fee{ + Amount: tc.amount, + } + + // Test that the encoder works with legacy_coins (which uses NullSliceAsEmptyEncoder) + bz, err := encoder.Marshal(fee) + require.NoError(t, err) + + var result map[string]interface{} + require.NoError(t, json.Unmarshal(bz, &result)) + + amountJSON, err := json.Marshal(result["amount"]) + require.NoError(t, err) + if tc.wantJSON == "" { + // For non-empty slices, just verify it's a valid array with expected content + require.Contains(t, string(amountJSON), "uatom") + require.Contains(t, string(amountJSON), "stake") + require.Contains(t, string(amountJSON), "1000") + require.Contains(t, string(amountJSON), "500") + } else { + require.Equal(t, tc.wantJSON, string(amountJSON), tc.description) + } + }) + } +} + +func TestNullSliceAsEmptyEncoderDirect(t *testing.T) { + encoder := aminojson.NewEncoder(aminojson.EncoderOptions{}) + + // Test direct usage of NullSliceAsEmptyEncoder with a custom field encoding + customEncoder := encoder.DefineFieldEncoding("test_field", aminojson.NullSliceAsEmptyEncoder) + require.NotNil(t, customEncoder) + + // Create a Fee message with an empty list (Fee uses legacy_coins which uses NullSliceAsEmptyEncoder) + fee := &txv1beta1.Fee{ + Amount: []*basev1beta1.Coin{}, // empty slice + } + + // Marshal using the encoder + bz, err := encoder.Marshal(fee) + require.NoError(t, err) + + var result map[string]interface{} + require.NoError(t, json.Unmarshal(bz, &result)) + + // Verify that amount field exists and is an empty array (not null) + amount, ok := result["amount"] + require.True(t, ok, "amount field should exist") + amountJSON, err := json.Marshal(amount) + require.NoError(t, err) + require.Equal(t, `[]`, string(amountJSON), "empty slice should be encoded as [] not null") +} diff --git a/x/tx/signing/aminojson/encoder.go b/x/tx/signing/aminojson/encoder.go index 4a20ae29be..95bd4addca 100644 --- a/x/tx/signing/aminojson/encoder.go +++ b/x/tx/signing/aminojson/encoder.go @@ -72,9 +72,22 @@ func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error { } } -// nullSliceAsEmptyEncoder replicates the behavior at: +// NullSliceAsEmptyEncoder replicates the behavior at: // https://github.com/cosmos/cosmos-sdk/blob/be9bd7a8c1b41b115d58f4e76ee358e18a52c0af/types/coin.go#L199-L205 -func nullSliceAsEmptyEncoder(enc *Encoder, v protoreflect.Value, w io.Writer) error { +// +// This encoder ensures that nil slices are encoded as empty arrays ([]) instead of null. +// This is useful for maintaining backward compatibility with legacy Amino JSON encoding +// where nil slices were serialized as empty arrays. +// +// Usage example: +// +// encoder := aminojson.NewEncoder(options) +// encoder = encoder.DefineFieldEncoding("my_field", aminojson.NullSliceAsEmptyEncoder) +// +// Or in protobuf: +// +// repeated MyType field = 1 [(amino.encoding) = "null_slice_as_empty"]; +func NullSliceAsEmptyEncoder(enc *Encoder, v protoreflect.Value, w io.Writer) error { switch list := v.Interface().(type) { case protoreflect.List: if list.Len() == 0 { diff --git a/x/tx/signing/aminojson/encoder_test.go b/x/tx/signing/aminojson/encoder_test.go index 42f47638d1..c2e540a4bc 100644 --- a/x/tx/signing/aminojson/encoder_test.go +++ b/x/tx/signing/aminojson/encoder_test.go @@ -7,6 +7,9 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/reflect/protoreflect" "gotest.tools/v3/assert" + + basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" + txv1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1" ) func TestCosmosInlineJSON(t *testing.T) { @@ -172,3 +175,73 @@ func TestSortedJSONStringify(t *testing.T) { }) } } + +func TestNullSliceAsEmptyEncoder(t *testing.T) { + encoder := NewEncoder(EncoderOptions{}) + + t.Run("empty list encodes as empty array", func(t *testing.T) { + // Create a Fee message with empty amount list + fee := &txv1beta1.Fee{ + Amount: []*basev1beta1.Coin{}, // empty slice + } + + // Get the list value from the message + msg := fee.ProtoReflect() + field := msg.Descriptor().Fields().ByName("amount") + listValue := msg.Get(field) + + // Test the encoder function directly + var buf bytes.Buffer + err := NullSliceAsEmptyEncoder(&encoder, listValue, &buf) + require.NoError(t, err) + assert.Equal(t, "[]", buf.String(), "Empty list should encode as [] not null") + }) + + t.Run("nil list encodes as empty array", func(t *testing.T) { + // Create a Fee message with nil amount + fee := &txv1beta1.Fee{ + Amount: nil, // nil slice + } + + // Get the list value from the message + msg := fee.ProtoReflect() + field := msg.Descriptor().Fields().ByName("amount") + listValue := msg.Get(field) + + // Test the encoder function directly + var buf bytes.Buffer + err := NullSliceAsEmptyEncoder(&encoder, listValue, &buf) + require.NoError(t, err) + assert.Equal(t, "[]", buf.String(), "Nil list should encode as [] not null") + }) + + t.Run("non-empty list encodes normally", func(t *testing.T) { + // Create a Fee message with non-empty amount list + fee := &txv1beta1.Fee{ + Amount: []*basev1beta1.Coin{ + {Denom: "uatom", Amount: "1000"}, + }, + } + + // Get the list value from the message + msg := fee.ProtoReflect() + field := msg.Descriptor().Fields().ByName("amount") + listValue := msg.Get(field) + + // Test the encoder function directly + var buf bytes.Buffer + err := NullSliceAsEmptyEncoder(&encoder, listValue, &buf) + require.NoError(t, err) + // Should encode the list normally (not just []) + require.Contains(t, buf.String(), "uatom") + require.Contains(t, buf.String(), "1000") + }) + + t.Run("unsupported type returns error", func(t *testing.T) { + // Test with unsupported type + var buf bytes.Buffer + err := NullSliceAsEmptyEncoder(&encoder, protoreflect.ValueOfString("not a list"), &buf) + require.Error(t, err) + require.Contains(t, err.Error(), "unsupported type") + }) +} diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 338ea9d88a..c153e1a35f 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -82,8 +82,9 @@ func NewEncoder(options EncoderOptions) Encoder { "threshold_string": thresholdStringEncoder, }, aminoFieldEncoders: map[string]FieldEncoder{ - "legacy_coins": nullSliceAsEmptyEncoder, - "inline_json": cosmosInlineJSON, + "legacy_coins": NullSliceAsEmptyEncoder, + "null_slice_as_empty": NullSliceAsEmptyEncoder, + "inline_json": cosmosInlineJSON, }, protoTypeEncoders: map[string]MessageEncoder{ "google.protobuf.Timestamp": marshalTimestamp,