Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Handlers constructed only once #2529

Closed
mossid opened this issue Oct 18, 2018 · 2 comments
Closed

Make Handlers constructed only once #2529

mossid opened this issue Oct 18, 2018 · 2 comments

Comments

@mossid
Copy link
Contributor

mossid commented Oct 18, 2018

The reason we are not implementing handler like Keeper.Handle() is that a malicious module can forge a msg and pass it to other keeper's handler to manipulate the state illegally if they hold the keeper. But still they can call NewHandler() that the modules are exposing.

Proposal

Define a function UniqueHandler:

var created map[uintptr]struct{} = make(map[uintptr]struct{})

func UniqueHandler(h sdk.Handler) sdk.Handler {
  ptr := reflect.ValueOf(h).Pointer()
  if _, ok := created[ptr]; ok {
    panic("Multiple generation of handler")
  }
  created[ptr] = struct{}{}
  return h
}

The users can simply wrap the inner handler in their NewHandler to prevent double-NewHandler

func NewHandler(k Keeper) sdk.Handler {
  return sdk.UniqueHandler(func(ctx sdk.Context, msg sdk.Msg) sdk.Result {
    //...
  }
}

In go playground: https://play.golang.org/p/D5QMF02KRAk

The downside of this implementation is that it willnot generate multiple handlers for multiple keepers within a module. For now the developer can simply don't use this function when they need multiple handlers.

@mossid mossid changed the title Make Handler construct only once Make Handlers constructed only once Oct 18, 2018
@rigelrozanski
Copy link
Contributor

Are we actually concerned about this?
My understanding was that we just wanted to avoid bad practices if modules were to call the handlers on the keepers within other modules.

Any module can still just broadcast the message if they want to manipulate the state.

@jackzampolin
Copy link
Member

This is no longer applicable, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants