improve proto3 JSON types and utils

[turadg]

Description

Spurred by some recent reviews.

• clarify the Any protobuf encoding
• use it where we can, to align with external types.
• move the vat-safe encoding concern lower than orchestration (applicable to all inter-vat comms)

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

Less to document

Testing Considerations

CI

Upgrade Considerations

not yet in production

[michaelfig]

I think this change requires more explicit understanding of why Base64 is required:

• If you declare a Golang struct to have a []byte member, then when you json.Marshal or json.Unmarshal it, the encoding/json Golang module converts []byte to and from base64.
• AnyKindOfProtoCodec.toJSON(object), including Any.toJSON(object) returns a "message" with byte arrays converted to Base64
• AnyKindOfProtoCodec.fromJSON(message), including Any.fromJSON(message) returns an object with Base64 converted to byte arrays
• That, to me, suggests that the type should be named something like AnyJson and defined as { typeUrl: string, value: string }, since that is just the Jsonable version of type Any = { typeUrl: string, value: Uint8Array }

[michaelfig]

cf cosmic-proto/dist/codegen/google/protobuf/any.js

[turadg]

The names Any and AnyJson make sense to me. I was hoping to distinguish between them structurely by their keys, but I forgot about this precedence that should be maintained:

agoric-sdk/packages/cosmic-proto/src/codegen/google/protobuf/any.ts

Lines 260 to 269 in 2973951

	toJSON(message: Any): unknown {
	const obj: any = {};
	message.typeUrl !== undefined && (obj.typeUrl = message.typeUrl);
	message.value !== undefined &&
	(obj.value = base64FromBytes(
	message.value !== undefined ? message.value : new Uint8Array(),
	));
	return obj;
	},
	fromPartial(object: Partial<Any>): Any {

Fortunately TS can still distinguish between them.

I wonder why that method returns unknown instead of { typeUrl: string, value: string }.

The two codecs in this file, we don't need them, right? It's just Any.fromJSON and Any.toJSON. I'll give that a whirl

[michaelfig]

I wonder why that method returns unknown

Maybe because just because the transpiler is being slothful. A proper toJSON return type would need to be prepared to either introduce a new type, or inline the type for an object that could potentially have many properties.

The two codecs in this file, we don't need them, right? It's just Any.fromJSON and Any.toJSON.

That's correct.

[turadg]

Meanwhile I've added a helper:

// TODO make codegen toJSON() return these instead of unknown
/**
 * Proto Any with arrays encoded as base64
 */
export type Base64Any<T> = {
  [Prop in keyof T]: T[Prop] extends Uint8Array ? string : T[Prop];
};

[cloudflare-workers-and-pages]

Deploying agoric-sdk with    Cloudflare Pages

Latest commit:	c77c1be
Status:	✅  Deploy successful!
Preview URL:	https://2f88cbbf.agoric-sdk.pages.dev
Branch Preview URL:	https://ta-json3.agoric-sdk.pages.dev

View logs

[0xpatrickdev]

We will also need this for RequestQuery.toJSON() (/tendermint/abci/types.js). Any chance it can fit in this PR?

[turadg]

that would require patching the codegen. out of scope, thus the TODO

[0xpatrickdev]

Not sure how this is related to the rest of the changes

[turadg]

type bug discovered in the course of the work: d68596cb8d1d84db4c4eb9af754b132af1ce1649