[Bug]: DefineCustomGetSigners is unusable.

[SpicyLemon]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

There is no way to use DefineCustomGetSigners on a Msg.

The DefineCustomGetSigners function uses the Message interface defined in "google.golang.org/protobuf/proto". That's an alias for the protoreflect.ProtoMessage interface, which just requires the ProtoReflect() Message method. But code generated from protos does not create that method on anything.

All of the unit tests related to signing uses the pulsar stuff. But those things aren't what are actually used by the msg server. So there's no way to define a custom GetSigners method for anything that someone would be actually signing.

Cosmos SDK Version

0.50.5

How to reproduce?

Here's the simplified situation I'm in where I need to define a custom GetSigners method.

service Msg {
  rpc StartThing(MsgStartThingRequest) MsgStartThingResponse;
  rpc EndThing(MsgEndThingRequest) MsgEndThingResponse;
}

message MsgStartThingRequest {
  Thing thing = 1;
}

message MsgStartThingResponse {}

message MsgEndThingRequest {
  Thing thing = 1;
}

message MsgEndThingResponse {}

message Thing {
  string addr1 = 1  [(cosmos_proto.scalar) = "cosmos.AddressString"];
  string addr2 = 2  [(cosmos_proto.scalar) = "cosmos.AddressString"];
}

The signer of MsgStartThingRequest should be thing.addr1.
The signer of MsgEndThingRequest should be thing.addr2.

I can't use option (cosmos.msg.v1.signer) here because it no longer allows sub-fields. I.e., in MsgStartThingRequest, I can't have option (cosmos.msg.v1.signer) = "thing.addr1", I can only have option (cosmos.msg.v1.signer) = "thing". I'd then have to put an option (cosmos.msg.v1.signer) in the Thing message, but it can't be hard-coded like that because the field that's the signer is different depending on which Msg the Thing is in. And there are other messages that also have a Thing where neither should be a signer.

In the go code, the msg server endpoint methods require the use of the MsgStartThingRequest struct as defined in the auto-generated go code that lives in the module code. Those Msg structs (where we used to have to define GetSigners()), do not have a ProtoReflect() method, though. So it's impossible to define a custom GetSigners function on a Msg required by an endpoint.

Here's what the go code might look like to define the custom GetSigners for MsgStartThingRequest:

	options.DefineCustomGetSigners(proto.MessageName(&MsgStartThingRequest{}), func(msgIn proto.Message) ([][]byte, error) {
		msg, ok := msgIn.(*MsgStartThingRequest)
		if !ok {
			return nil, fmt.Errorf("incorrect message type, actual: %T, expected: %T", msgIn, &MsgStartThingRequest{})
		}
		addr, err := options.AddressCodec.StringToBytes(msgCP.Thing.Addr1)
		if err != nil {
			return nil, err
		}
		return [][]byte{addr}, nil
	})

In that, options is a cosmossdk.io/x/tx/signing.Options, and proto is the google.golang.org/protobuf/proto package.

There are two compilation issues with that code.

1. proto.MessageName takes in a protoreflect.ProtoMessage, but MsgStartThingRequest doesn't have a ProtoReflect() method, so &MsgStartThingRequest{} is an invalid argument for that function.
2. The proto.Message type that the function takes in, is just an alias for that protoreflect.ProtoMessage. So msgIn cannot be asserted as a (*MsgStartThingRequest) because again, MsgStartThingRequest does not have a ProtoReflect() method.

[kocubinski]

In the above example you need to run the pulsar code generator for MsgStartThingRequest and all other messages. Once they're generated you can use options.DefineCustomGetSigners with those types.

The gogo types are converted to their wire format and dynamicpb when used in x/tx/signer code to convert from gogo -> a ProtoReflect enabled message.

[SpicyLemon]

Having to create the pulsar code generation process just so that a custom GetSigners function can be defined is a pretty heavy requirement. It's also not a very intuitive thing to have to use. Everything else related to the endpoints and their Msgs is defined based on the standard generated code. Until v0.50, we had to define a GetSigners function on each of those Msg types in order for them to work at all. It makes more sense to still define what's needed based on those Msg types instead of having to define them on some other version of them that isn't use anywhere else.

I did figure out how to do it without the pulsar code, but it's pretty annoyingly convoluted as it has to rely on the proto reflection stuff instead of being able to just access the field like I would anywhere else in the code. Even with the pulsar code, though, I'm not sure it'd get any better.

Instead of proto.MessageName(&MsgStartThingRequest{}), I had to use proto.MessageName(protoadapt.MessageV2Of(&MsgStartThingRequest{})) (protoadapt is the google.golang.org/protobuf/protoadapt package).

Then, in the actual GetSignersFunc, I had to us proto reflection to first get the thing from the Msg, then the desired field from that.

Here's what it ended up looking like:

func createThingGetSignersFunc(options *signing.Options, fieldName string) signing.GetSignersFunc {
    return func(msgIn proto.Message) (addrs [][]byte, err error) {
        defer func() {
            if r := recover(); r != nil {
                err = fmt.Errorf("panic (recovered) getting %s.thing.%s as a signer: %v", proto.MessageName(msgIn), fieldName, r)
            }
        }()

        msg := msgIn.ProtoReflect()
        thingDesc := msg.Descriptor().Fields().ByName("thing")
        if thingDesc == nil {
            return nil, fmt.Errorf("no thing field found in %s", proto.MessageName(msgIn))
        }

        thing := msg.Get(thingDesc).Message()
        fieldDesc := thing.Descriptor().Fields().ByName(protoreflect.Name(fieldName))
        if fieldDesc == nil {
            return nil, fmt.Errorf("no thing.%s field found in %s", fieldName, proto.MessageName(msgIn))
        }

        b32 := thing.Get(fieldDesc).Interface().(string)
        addr, err := options.AddressCodec.StringToBytes(b32)
        if err != nil {
            return nil, fmt.Errorf("error decoding thing.%s address %q: %w", fieldName, b32, err)
        }
        return [][]byte{addr}, nil
    }
}

func DefineCustomGetSigners(options *signing.Options) {
    options.DefineCustomGetSigners(
        proto.MessageName(protoadapt.MessageV2Of(&MsgStartThingRequest{})),
        createThingGetSignersFunc(options, "addr1"))
    options.DefineCustomGetSigners(
        proto.MessageName(protoadapt.MessageV2Of(&MsgEndThingRequest{})),
        createThingGetSignersFunc(options, "addr2"))
}

That DefineCustomGetSigners function is then called in app.New when creating the rest of the encoding stuff.

[SpicyLemon]

Feel free to close this bug. It'd be nice if this stuff were at least documented somewhere, though.

Even better if I could just do option (cosmos.msg.v1.signer) = "thing.addr1" in the Msg. It's just the Msg that's concerned about who the signer is, so it makes more sense to be able to fully define the signer in there rather than having to have signer options in non-Msg messages. I appreciate the complexity of allowing that, though, and such a change probably not worth the effort.