feat(x/tx): Rename custom Amino JSON encoder to "inline_json" (cosmos…

…#19919)

Co-authored-by: Matt Kocubinski <[email protected]>

docs/build/building-modules/05-protobuf-annotations.md

@@ -126,8 +126,21 @@ Encoding instructs the amino json marshaler how to encode certain fields that ma
 https://github.com/cosmos/cosmos-sdk/blob/e8f28bf5db18b8d6b7e0d94b542ce4cf48fed9d6/proto/cosmos/bank/v1beta1/genesis.proto#L23
 ```
 
-Another example is how `bytes` is encoded when using the amino json encoding format. The `bytes_as_string` option tells the json marshaler [how to encode bytes as string](https://github.com/pinosu/cosmos-sdk/blob/9879ece09c58068402782fa2096199dc89a23d13/x/tx/signing/aminojson/json_marshal.go#L75).
+Another example is a protobuf `bytes` that contains a valid JSON document.
+The `inline_json` option tells the json marshaler to embed the JSON bytes into the wrapping document without escaping.
 
 ```proto
-(amino.encoding)         = "bytes_as_string",
-```
+(amino.encoding)         = "inline_json",
+```
+
+E.g. the bytes containing `{"foo":123}` in the `envelope` field would lead to the following JSON:
+
+```json
+{
+  "envelope": {
+    "foo": 123
+  }
+}
+```
+
+If the bytes are not valid JSON, this leads to JSON broken documents. Thus a JSON validity check needs to be in place at some point of the process.

x/tx/CHANGELOG.md

@@ -33,7 +33,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
-* [#19786](https://github.com/cosmos/cosmos-sdk/pull/19786) Add bytes as string option to encoder.
+* [#19786](https://github.com/cosmos/cosmos-sdk/pull/19786)/[#19919](https://github.com/cosmos/cosmos-sdk/pull/19919) Add "inline_json" option to Amino JSON encoder.
 
 ### Improvements
 

x/tx/signing/aminojson/encoder.go

@@ -81,16 +81,21 @@ func nullSliceAsEmptyEncoder(enc *Encoder, v protoreflect.Value, w io.Writer) er
 	}
 }
 
-// cosmosBytesAsString replicates the behavior at:
+// cosmosInlineJSON takes bytes and inlines them into a JSON document.
+//
+// This requires the bytes contain valid JSON since otherwise the resulting document would be invalid.
+// Invalid JSON will result in an error.
+//
+// This replicates the behavior of JSON messages embedded in protobuf bytes
+// required for CosmWasm, e.g.:
 // https://github.com/CosmWasm/wasmd/blob/08567ff20e372e4f4204a91ca64a371538742bed/x/wasm/types/tx.go#L20-L22
-func cosmosBytesAsString(_ *Encoder, v protoreflect.Value, w io.Writer) error {
+func cosmosInlineJSON(_ *Encoder, v protoreflect.Value, w io.Writer) error {
 	switch bz := v.Interface().(type) {
 	case []byte:
-		blob, err := json.RawMessage(bz).MarshalJSON()
-		if err != nil {
-			return err
+		if !json.Valid(bz) {
+			return errors.New("invalid JSON bytes")
 		}
-		_, err = w.Write(blob)
+		_, err := w.Write(bz)
 		return err
 	default:
 		return fmt.Errorf("unsupported type %T", bz)

x/tx/signing/aminojson/encoder_test.go

@@ -9,21 +9,64 @@ import (
 	"gotest.tools/v3/assert"
 )
 
-func TestCosmosBytesAsString(t *testing.T) {
+func TestCosmosInlineJSON(t *testing.T) {
 	cases := map[string]struct {
 		value      protoreflect.Value
 		wantErr    bool
 		wantOutput string
 	}{
-		"valid bytes - json": {
+		"supported type - valid JSON object": {
 			value:      protoreflect.ValueOfBytes([]byte(`{"test":"value"}`)),
 			wantErr:    false,
 			wantOutput: `{"test":"value"}`,
 		},
-		"valid bytes - string": {
-			value:      protoreflect.ValueOfBytes([]byte(`foo`)),
+		"supported type - valid JSON array": {
+			// spaces are normalized away
+			value:      protoreflect.ValueOfBytes([]byte(`[1,2,3]`)),
 			wantErr:    false,
-			wantOutput: `foo`,
+			wantOutput: `[1,2,3]`,
+		},
+		"supported type - valid JSON is not normalized": {
+			value:      protoreflect.ValueOfBytes([]byte(`[1, 2, 3]`)),
+			wantErr:    false,
+			wantOutput: `[1, 2, 3]`,
+		},
+		"supported type - valid JSON array (empty)": {
+			value:      protoreflect.ValueOfBytes([]byte(`[]`)),
+			wantErr:    false,
+			wantOutput: `[]`,
+		},
+		"supported type - valid JSON number": {
+			value:      protoreflect.ValueOfBytes([]byte(`43.72`)),
+			wantErr:    false,
+			wantOutput: `43.72`,
+		},
+		"supported type - valid JSON boolean": {
+			value:      protoreflect.ValueOfBytes([]byte(`true`)),
+			wantErr:    false,
+			wantOutput: `true`,
+		},
+		"supported type - valid JSON null": {
+			value:      protoreflect.ValueOfBytes([]byte(`null`)),
+			wantErr:    false,
+			wantOutput: `null`,
+		},
+		"supported type - valid JSON string": {
+			value:      protoreflect.ValueOfBytes([]byte(`"hey yo"`)),
+			wantErr:    false,
+			wantOutput: `"hey yo"`,
+		},
+		"supported type - invalid JSON": {
+			value:   protoreflect.ValueOfBytes([]byte(`foo`)),
+			wantErr: true,
+		},
+		"supported type - invalid JSON (empty)": {
+			value:   protoreflect.ValueOfBytes([]byte(``)),
+			wantErr: true,
+		},
+		"supported type - invalid JSON (nil bytes)": {
+			value:   protoreflect.ValueOfBytes(nil),
+			wantErr: true,
 		},
 		"unsupported type - bool": {
 			value:   protoreflect.ValueOfBool(true),
@@ -38,7 +81,7 @@ func TestCosmosBytesAsString(t *testing.T) {
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			var buf bytes.Buffer
-			err := cosmosBytesAsString(nil, tc.value, &buf)
+			err := cosmosInlineJSON(nil, tc.value, &buf)
 
 			if tc.wantErr {
 				require.Error(t, err)

x/tx/signing/aminojson/internal/testpb/test.proto

@@ -20,6 +20,10 @@ message WithAList {
   repeated string list                = 2;
 }
 
+message WithAJson {
+  bytes field1 = 1 [(amino.encoding) = "inline_json"];
+}
+
 message ABitOfEverything {
   option (amino.name) = "ABitOfEverything";