Code genertated from optional message field is mission optionality

[webmaster128]

I have the following protobuf definition:

message MsgStoreCode {
  option (amino.name) = "wasm/MsgStoreCode";
  option (cosmos.msg.v1.signer) = "sender";

  // Sender is the actor that signed the messages
  string sender = 1;
  // WASMByteCode can be raw or gzip compressed
  bytes wasm_byte_code = 2 [ (gogoproto.customname) = "WASMByteCode" ];
  // Used in v1beta1
  reserved 3, 4;
  // InstantiatePermission access control to apply on contract creation,
  // optional
  AccessConfig instantiate_permission = 5;
}

After upgrading to Telescope 0.109.0, the instantiate_permission got non-optional:

However, this change is wrong as by default all message fields in protobuf should be optional. Only the ones annotated as non-optional should be non-toptional.

So the desired output for the above proto is:

/** MsgStoreCode submit Wasm code to the system */
export interface MsgStoreCode {
  /** Sender is the actor that signed the messages */
  sender: string;
  /** WASMByteCode can be raw or gzip compressed */
  wasmByteCode: Uint8Array;
  /**
   * InstantiatePermission access control to apply on contract creation,
   * optional
   */
-  instantiatePermission: AccessConfig;
+  instantiatePermission?: AccessConfig;
}

This bug currently leads to "instantiate permission: type: empty" errors in cosmos/cosmjs#1484 because the field is set but empty and not unset as expected.

[Zetazzz]

Thanks! I'm on it.

[Zetazzz]

Hi, fixed with this PR: #476

After the PR's published, this setting can solve the issue:
prototypes: {
fieldDefaultIsOptional: true,
useOptionalNullable: true, // false, if don't want to use [(gogoproto.nullable) = false]
}

proto:

message EvalRequest {
  // Bindings for the external variables.  The types SHOULD be compatible
  // with the type environment in [CheckRequest][google.api.expr.conformance.v1alpha1.CheckRequest], if checked.
  map<string, google.api.expr.v1alpha1.ExprValue> bindings = 1;

  map<string, google.api.expr.v1alpha1.IdRef> refs = 2;

  uint32 test_num = 3;
  string test_string = 4[(gogoproto.nullable) = false];
  bool test_bool = 5;

  // Used in v1beta1
  reserved 6, 7, 9 to 11;

  AccessConfig instantiate_permission = 8;

  oneof test_oneof {
    // [(gogoproto.nullable) = false] wouldn't work in this case
    string id = 12[(gogoproto.nullable) = false];
    string name = 13;
  }

  repeated string test_array = 14;
}

ts:

export interface EvalRequest {
  /**
   * Bindings for the external variables.  The types SHOULD be compatible
   * with the type environment in [CheckRequest][google.api.expr.conformance.v1alpha1.CheckRequest], if checked.
   */
  bindings?: {
    [key: string]: ExprValue;
  };
  refs?: {
    [key: string]: IdRef;
  };
  testNum?: number;
  //this field's required becuse [(gogoproto.nullable) = false];
  //if useOptionalNullable set to false, this'll be optional too.
  testString: string;
  testBool?: boolean;
  instantiatePermission?: AccessConfig;
  /** [(gogoproto.nullable) = false] wouldn't work in this case, since fields'll be optional in oneof anyway */
  id?: string;
  name?: string;
  testArray?: string[];
}

If you think this solution's ok, I'll merge and publish

Thank you very much!

[Zetazzz]

I believe the unexpected results was because of Telescope consider the default value of (gogoproto.nullable) is false, which should be true.

After this change, the default value will be true and the result:

Proro:

message EvalRequest {
  // Bindings for the external variables.  The types SHOULD be compatible
  // with the type environment in [CheckRequest][google.api.expr.conformance.v1alpha1.CheckRequest], if checked.
  map<string, google.api.expr.v1alpha1.ExprValue> bindings = 1;

  map<string, google.api.expr.v1alpha1.IdRef> refs = 2;

  uint32 test_num = 3;
  string test_string = 4[(gogoproto.nullable) = false];
  bool test_bool = 5;

  // Used in v1beta1
  reserved 6, 7, 9 to 11;

  AccessConfig instantiate_permission = 8;

  oneof test_oneof {
    // [(gogoproto.nullable) = false] wouldn't work in this case
    string id = 12[(gogoproto.nullable) = false];
    string name = 13;
  }

  repeated string test_array = 14;
}

will generate TS:

export interface EvalRequest {
  /**
   * Bindings for the external variables.  The types SHOULD be compatible
   * with the type environment in [CheckRequest][google.api.expr.conformance.v1alpha1.CheckRequest], if checked.
   */
  bindings: {
    [key: string]: ExprValue;
  };
  refs: {
    [key: string]: IdRef;
  };
  testNum: number;
  testString: string;
  testBool: boolean;
  instantiatePermission?: AccessConfig;
  /** [(gogoproto.nullable) = false] wouldn't work in this case, since it's inside oneof */
  id?: string;
  name?: string;
  testArray: string[];
}

telescope 1.0.4 has been published with this fix

[webmaster128]

I believe the unexpected results was because of Telescope consider the default value of (gogoproto.nullable) is false, which should be true.

Yeah, the [(gogoproto.nullable) = false] annotation only exist because by default all message fields are nullable.

Thank you for the fix! Looks good so far