Change GetSigners to return a single address as string

[amaury1093]

Summary

In the sdk.Msg interface, change GetSigners to always return one signer, as a bech32-encoded string.

Problem Definition

There has been discussion (see here and Discord) to keep 1 Msg == 1 signer. The only Msg that doesn't do that is MsgMultiSend, but the same behavior can be achieved with a multi-Msg tx, see #12601.

In #9239, we also considered changing GetSigners to []string to remove the bech32 config dependency.

Proposal

Combine the 2 issues above, and make GetSigners always return 1 signer as a bech32-encoded string.

	Msg interface {
		proto.Message
		ValidateBasic() error
-		GetSigners() []AccAddress
+		GetSigner() string
	}

Altnernative: Only add the new method, and deprecate the old one

We can consider only adding a new GetSigner() string method to that interface, and mark as deprecated the existing GetSigners() []AccAddress, which might make the transition smoother. But I'd argue it's anyways an API-breaking change, so let's do it only once and for good.

---

For Admin Use

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

[robert-zaremba]

Why making that limitation? What's the benefit? GetSigners is going to be automated with proto annotations anyway.

Maybe other, custom messages require multiple signers?

BTW: I agree that it's API breaking.

[robert-zaremba]

If you want to remove bech32 dependency then we can change return type from []sdk.AccAddess to []string as it was done in #9418

[alexanderbez]

ACK @AmauryM -- let's just rip the bandaid off and modify the existing API to be GetSigner() string.

[sunnya97]

Wait, I agree with @robert-zaremba. Why are we removing the flexibility to allow for multiple signers? There may be Msgs in other chains that make use of this

The situation I brought up here was very specifically due to the non 1:1 nature of senders and recipients in that msg. I think many sender, single recipient msg type would also be useful, and that needs the ability to be multi signer.

[0Tech]

Any updates on this? Theoretically, this change would harm the flexibility, but I couldn't come up with any practical usage. It seems that the last candidate against this now has been removed (on #14567).

Also, you should consider the limitation in x/authz, which prohibits MsgExec on messages of multiple signer. So if we decide to keep the feature, we may also have to add some improvement into the design of x/authz.

I'm not talking about the API. The semantics matters.

[robert-zaremba]

There could be MsgMultisignExec

[aaronc]

I think we probably shouldn't do this and close this issue, given the breakage and effort required

[0Tech]

There could be MsgMultisignExec

And, the key format of the grants is granter/grantee/msg_type for now, so we should also revisit the format. Or we may need to introduce a new key prefix for the multiple signer grants.

I think we probably shouldn't do this and close this issue, given the breakage and effort required

Then, what's the cosmos-sdk's position on multiple signer messages?

1. cosmos-sdk neither forbids it nor supports it. So we can let x/authz as is.
2. cosmos-sdk does forbid it, but retain the relevant APIs. We should document it somewhere and add the validations.
3. cosmos-sdk fully supports it. So we must update x/authz and other relevant logic later. Also, new designs should also take this into account.

[tac0turtle]

I think the proto approach should be GetSigner and we have a way to override it to GetSigners, but the sdk will revert to GetSigner for all things. The multimsg approach solves most usecases or at least i havent heard of one that needs this feature

[amaury1093]

Pasting my Slack comment here too:

Since we're moving to proto annotations signers, we can do:

• keep GetSigners like before (assume it ways returns 1 element)
• but in the new protoreflect-based GetSignersContext, only allow one signer

Also, the cosmos.msg.v1.signer validation will always check that it points to a single string field.

[aaronc]

Just for the record, I am opposed to this proposal. If I were designing the SDK from scratch I would probably only allow one signer but I don't see the pain points here being sufficient justification for removing the flexibility and causing a protocol level breaking change.

One valid use case is group MsgSubmitProposal which allows multiple group members to propose, approve and execute an operation without introducing any transient group proposal state. This simulates the way the current multisig pubkey works and I don't see a way to achieve this same behavior without multi signers.

[tac0turtle]

discussed this to great lengths in a few chats,

The final outcome was we wont remove this functionality, thanks to everyone who participated in these chats