Rename custom Amino JSON encoder to inline_json

and increase test coverage

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/signing/aminojson/encoder.go

@@ -81,11 +81,16 @@ 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 assumes the bytes contain valid JSON since otherwise the resulting document is invalid.
+//
+// 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:
+		// Caution, this does not include a validity check as json.RawMessage can be anything 🤷‍♂️
 		blob, err := json.RawMessage(bz).MarshalJSON()
 		if err != nil {
 			return err

x/tx/signing/aminojson/encoder_test.go

@@ -9,18 +9,59 @@ 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": {
+		"supported type - valid JSON array": {
+			// spaces are normalized away
+			value:      protoreflect.ValueOfBytes([]byte(`[1,2,3]`)),
+			wantErr:    false,
+			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"`,
+		},
+		// This test case is a bit tricky. Conceptually it makes no sense for this
+		// to pass. The question is just where the JSON validity check is done.
+		// In wasmd we have it in the message field validation. But it might make
+		// sense to move it here instead.
+		// For now it's better to consider this case undefined behaviour.
+		"supported type - invalid JSON": {
 			value:      protoreflect.ValueOfBytes([]byte(`foo`)),
 			wantErr:    false,
 			wantOutput: `foo`,
@@ -38,7 +79,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/json_marshal.go

@@ -72,8 +72,8 @@ func NewEncoder(options EncoderOptions) Encoder {
 			"threshold_string": thresholdStringEncoder,
 		},
 		aminoFieldEncoders: map[string]FieldEncoder{
-			"legacy_coins":    nullSliceAsEmptyEncoder,
-			"bytes_as_string": cosmosBytesAsString,
+			"legacy_coins": nullSliceAsEmptyEncoder,
+			"inline_json":  cosmosInlineJSON,
 		},
 		protoTypeEncoders: map[string]MessageEncoder{
 			"google.protobuf.Timestamp": marshalTimestamp,