diff --git a/docs/build/building-modules/05-protobuf-annotations.md b/docs/build/building-modules/05-protobuf-annotations.md index 5eaee51b8139..fe212f726b0e 100644 --- a/docs/build/building-modules/05-protobuf-annotations.md +++ b/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", -``` \ No newline at end of file +(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. diff --git a/x/tx/signing/aminojson/encoder.go b/x/tx/signing/aminojson/encoder.go index 5b8a91f449d7..fdd1f2310f34 100644 --- a/x/tx/signing/aminojson/encoder.go +++ b/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 diff --git a/x/tx/signing/aminojson/encoder_test.go b/x/tx/signing/aminojson/encoder_test.go index a843238614bb..3bfa301c0723 100644 --- a/x/tx/signing/aminojson/encoder_test.go +++ b/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) diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index cdbb9aa2ac21..f276cb39d23e 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/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,