Refactor to delete every modules keeper/hooks.go file

[ValarDragon]

Summary

~Every module has many lines of code of pure boilerplate around hooks. This is two-fold:

• keeper/hooks.go, which basically wraps a call to a hook defined by an interface in the types package. Sometimes does a nil check.
• types/hooks.go Makes a MultiStakingHooks.go file, that just calls

Problem Definition

These are two files in every module, contributing to the overall boilerplate and cognitive overhead of the SDK. If we fix this, were saving 1000+ LOC across cosmos

Proposal

Eliminate keeper/hooks.go

This seems more straightforward, make the default value for keeper.hooks{} a non-nil no-op multi-hook. Then make every call just use k.hooks.{hook name} rather than k.{hook name}

Eliminate types/hooks.go

This one I'm less sure on how to do. If anyone has ideas would love to know. At minimum, we can make every multi-hook function a one-liner, rather than a 6 liner, by just using a helper function Map of type interface{}. See https://blog.burntsushi.net/type-parametric-functions-golang/ in Reflection in Go 1.0.x.

Then you combine that with run-time reflection to call a function on the struct by human readable name (https://stackoverflow.com/questions/8103617/call-a-struct-and-its-method-by-name-in-go), and the args.

One can either use generics to make the Map function expect a slice as type h or use generics for it.

So then what this would transform into is:
before:

func (h MultiStakingHooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
	for i := range h {
		if err := h[i].AfterValidatorCreated(ctx, valAddr); err != nil {
			return err
		}
	}

	return nil
}

after:

func (h MultiStakingHooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) error {
	return utils.Map(h, "AfterValidatorCreated", ctx, valAddr)
}

---

For Admin Use

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

[ValarDragon]

To be clear, I'm not fully convinced on the correctness of the deleting types/hooks.go / if that is right in the complexity vs code brevity tradeoff. I think there should exist a more clever solution that should allow eliminating that struct having to be manually defined entirely.

I do think deleting the keeper/hooks.go is a clear win, and will also force improvement in the API abstractions in various testing code.

[08d2]

Just because code of similar or identical structure exists in more than one place doesn't mean it's "boilerplate", and even if code is "boilerplate" that doesn't necessarily have a negative effect on a codebase. A little copying is better than a little dependency.

Your proposed solutions rely on reflection, which should be avoided unless absolutely necessary. In fact, reflection-based code is generally far more deleterious to cognitive complexity than boilerplate! Reflection is never clear.

[tac0turtle]

I do think deleting the keeper/hooks.go is a clear win, and will also force improvement in the API abstractions in various testing code.

agree here!! im a fan of doing it

[ValarDragon]

Your proposed solutions rely on reflection, which should be avoided unless absolutely necessary. In fact, reflection-based code is generally far more deleterious to cognitive complexity than boilerplate! Reflection is never clear.

Only the types/hooks.go relies on reflection! keeper/hooks.go being deleted does not. Agreed that its unclear whether the types/hooks.go elimination via reflection is worth it, and that theres probably a better solution.

[aaronc]

A reminder to everyone that we have already agreed in prior discussions that #9656 is the long-term replacement for hooks

[tac0turtle]

A reminder to everyone that we have already agreed in prior discussions that #9656 is the long-term replacement for hooks

while this is the long term solution I think removing some boiler plate today is a fine solution. If it takes longer than a day, which it shouldn't, then its a small win.