Cosmos-sdk: AddRoute: route name must match msg.Type

[ebuchman]

eg.
app.Router().AddRoute("bank", bank.NewHandler(bank.NewCoinKeeper(app.accountMapper)))

We should probably remove the degree of freedom of naming it since the messages in bank need to return the same name

[rigelrozanski]

Could we instead use infer the name from themsg.Type then? That would be nice.

[mossid]

I think the best options we have without making any modification to the api are:

app.Router().AddRoute(SendMsg{}.Type(), bank.NewHandler(ck))

or

func (msg ExampleMsg) NewHandler(ck CoinKeeper) sdk.Handler {
    return newHandler(ck)
}
//--------------------------------------
app.Router().AddRoute(SendMsg{}.Type(), SendMsg{}.NewHandler(ck))

[ebuchman]

maybe we can expose bank.MsgType ? and then use that consistently with everything?

Alternatively we'd make Handler something that exposes a Type method.

[rigelrozanski]

@ebuchman, @mossid .. So each module creates a high level interface which is registered with the router and which is returned by each messages Type() function within a module? .......... I'm not sure we're getting around any of the complexity really.

Just and idea: we could create a handler interface (HandleI) to the effects of something like so:

type HandlerFunc func(ctx Context, msg Msg) Result

type Handler interface {
    GetFunc() HandlerFunc
    Type() interface{} 
}

type bankHandler struct {
    ck CoinKeeper
}

func (h bankHandler) GetFunc() HandlerFunc {
    return func....
}

func (_ bankHandler) Type() MsgType {
    ...
}

// to enforce
var _ Handler = bankHandler{Somecoinkeeperobject}

That way we wouldn't need to pass in anything, it's just a property of the Handler object passed into the router:

app.Router().AddRoute(bank.bankHandler{bank.NewCoinKeeper(app.accountMapper)})

Is this way to obtuse? lol It's cool - and it works - check it out this rough example:

https://play.golang.org/p/XB4LwnkqjJw

[ebuchman]

I think

type Handler interface {
    Handle(ctx Context, msg Msg) Result
    Type() interface{} 
}

might be nicer, but yeh, that's the idea.

[ValarDragon]

Why are we making the return type of Handler.Type(), interface{}, as opposed to just making string? (Since that is what Msg.GetType() returns right now)

[rigelrozanski]

@ValarDragon so then we don't need to even need to have String() within each message - the actual message struct can act as the type ... (I think)

[mossid]

type Handler interface {
    Handle(ctx Context, msg Msg) Result
    Type() interface{} 
}

If this is the final design I can start work on it.

[ebuchman]

Oh i think it should probably just be a string right

not sure why it was suggested to be an interface{}, doesn’t make much sense unless we create new interfaces for each group of Msg in a module

[rigelrozanski]

Oh yeah, on the same page now, string makes more sense. Ideally we set the string only once in the module, which will be referred to by the module msgs as well as the handler. nice

[mossid]

How do you think about making keepers implement sdk.Handler, instead of declaring handler struct that only contains keeper?

[rigelrozanski]

It's a subtle change, but I think it's a good idea to keep the handlers separated out and NOT be a function of the keeper RE:A-Bunch-Of-Conversations-With- @jaekwon

I think the biggest reason we want them separated out is we don't by default want to allow other modules which are importing the keeper access to the handler functionality // modules should ideally only access the keeper core functions which should be abstracted out as necessary from the handler - right like we don't want another module constructing a message type to call functionality of a keeper, it should just be separated within that keeper.

In very specific circumstance where for some reason we actually do want a module to send messages - that special type of module could take in the router directly and call the other handlers through the router.

[mossid]

Closing(see https://github.com/cosmos/cosmos-sdk/pull/1346 comments)