proposal: add a new MessageOption extension in signing to mark messages expected signers.

[fdymylja]

Summary

This proposes to add a new google.protobuf.MessageOption extension in signing which can be used to mark an sdk.Msgs expected signer in a language agnostic way.

Problem Definition

The GetSigners method of a message depends on the golang implementation. This means that multichain dynamic clients cannot make use of this information and need to manually populate a TXs authentication info, same goes for clients written in a language which is not golang.

This is unscalable long term, and a big maintainance burden, and is a big blocker in the design of generalised multipurpose tooling for working with cosmos-sdk chains.

Proposal

This is backwards compatible.

In cosmos-sdk/proto/cosmos/tx/signing/v1beta1/signing.proto (or it could be another file):

We add the following protobuf extension:

extend google.protobuf.MessageOptions {
  // required_signers must contain the fields of the protobuf
  // message descriptor which is extend through this option
  // that represent the signers of a transaction.
  repeated string required_signers = 11100000;
}

Then in every tx.proto file of the sdk, we use the extension to mark which fields in a message identify a signer,

Example in cosmos-sdk/proto/cosmos/bank/v1beta1/tx.proto:

// MsgSend represents a message to send coins from one account to another.
message MsgSend {
  // this is the added message option which marks signers.
  option (cosmos.tx.signing.v1beta1.required_signers) = "from_address"; 
  
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string   from_address                    = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
  string   to_address                      = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
  repeated cosmos.base.v1beta1.Coin amount = 3
      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
}

Note: there are more complex signing structures (although not many) such as MsgMultiSend in which the signers are inside the Input message - for this specific case a solution would be to extend Input with the required_signers extension.

Besides this change in the proto files, another change is required for correct implementation which is that during MsgServers registration we also do proper checks on the descriptor that:

1. required_signers field exist in a message
2. required_signers field are all of kind string.

cc: @aaronc, @AmauryM, @jackzampolin, @marbar3778

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[amaury1093]

Thanks! This is a great addition, I love this. This should make client development easier (ccing @webmaster128 for fyi).

Note: there are more complex signing structures (although not many) such as MsgMultiSend in which the signers are inside the Input message - for this specific case a solution would be to extend Input with the required_signers extension.

Could you also write down how required_signers would look like in that case?

Also: why is this extension in cosmos, and not in cosmos_proto?

[webmaster128]

Great stuff, thanks a lot for pushing this! Right now we somehow expect the user to know who needs to sign a message or build UIs where only the correct person can sign due to limited message support. Great to see this information in the proto file.

[fdymylja]

thank you for the feedback @AmauryM.

Also: why is this extension in cosmos, and not in cosmos_proto?

Although I have no strong opinion in where the extension definition should be, I think signing is a better fit for two reasons:

1. I intended cosmos_proto as encoding/decoding information.
2. Tying this to signing feels more right because it is strictly related to how we sign things right now (emphasis on right now), and this extension will eventually evolve based on how signing evolves.

Could you also write down how required_signers would look like in that case?

In MsgMultiSend we specify that inputs contains a signer.

message MsgMultiSend {
  // we signal inputs contains signers, the implementation can see that inputs is repeated of a protoreflect.MessageKind
  option (cosmos.tx.signing.v1beta1.required_signers) = "inputs"; 
 
  option (gogoproto.equal) = false;

  repeated Input  inputs  = 1 [(gogoproto.nullable) = false];
  repeated Output outputs = 2 [(gogoproto.nullable) = false];
}

In Input we signal which is the real signer field.

message Input {
   // we signal in Input that address is a field that contains a signer
   option (cosmos.tx.signing.v1beta1.required_signers) = "address"; 
    
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string   address                        = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
  repeated cosmos.base.v1beta1.Coin coins = 2
      [(gogoproto.nullable) = false, (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"];
}

We can avoid signalling in MsgMultiSend that input contains a signer and let the DynamicGetSigners implementation expand the descriptors and find it by itself, but I still think explicit is better than implicit in this case.

[aaronc]

I was thinking of putting the option as a bool on the field itself:

message MsgSend {
  string   from_address = 1 [(cosmos.tx.v1.signer) = true];
  ...
}

Any reason to prefer one or the other? I guess the string approach explicitly specifies order, whereas order would be implicit (field number-based probably?) with the field option. With the string approach, you could misspell something or incorrectly specify - either way we'd want some runtime validation at startup.

This is related to deprecating the sdk.Msg interface as discussed in #10368 btw.

[fdymylja]

I was thinking of putting the option as a bool on the field itself:

message MsgSend {
  string   from_address = 1 [(cosmos.tx.v1.signer) = true];
  ...
}

Any reason to prefer one or the other? I guess the string approach explicitly specifies order, whereas order would be implicit (field number-based probably?) with the field option. With the string approach, you could misspell something or incorrectly specify - either way we'd want some runtime validation at startup.

This is related to deprecating the sdk.Msg interface as discussed in #10368 btw.

Runtime checks will be a must due to kind assertions.

I have no swtrong opinion on field option vs message option.

Field option pros:

1. it's not error prone on field naming.

Field option cons:

1. get signers impl will need to check every field recursively (maybe we could pre-compute it at runtime though?)

Message option pros:

1. Signals directly on the message which are the signer fields.

Message option cons:

1. More error prone (although, you do it once only).

If we reach consensus on this part I can start a PR to implement this, there are some libs that could make use of this feature right now (in the immediate term too).

[aaronc]

Thanks @fdymylja 🙏. I honestly don't have a strong opinion on field vs message. @AmauryM @webmaster128 ?

Let's please do this in a v1 proto package. Maybe cosmos.msg.v1?

[amaury1093]

Small preference for option (cosmos.tx.signing.v1beta1.required_signers) = "from_address";. It reads better for humans.

[aaronc]

option (cosmos.tx.signing.v1beta1.required_signers) = "from_address";

How about option (cosmos.msg.v1.signers) = "from_address"? I like shorter names when possible 😉

[fdymylja]

I will work on it this week or next.

Could we include this into 0.45 (proto changes + msg checks)?
Could we backport this into 0.44 (proto changes only)?