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

Module-Specific AnteHandlers #4572

Closed
4 tasks
AdityaSripal opened this issue Jun 17, 2019 · 4 comments · Fixed by #5006
Closed
4 tasks

Module-Specific AnteHandlers #4572

AdityaSripal opened this issue Jun 17, 2019 · 4 comments · Fixed by #5006
Assignees
Milestone

Comments

@AdityaSripal
Copy link
Member

AdityaSripal commented Jun 17, 2019

Summary

Allow modules to define module-specific antehandling functions (for now call these ModuleAnteHandler). Any module that has ModuleAnteHandler defined can get registered with ModuleManager. These ModuleAnteHandlers can then get run in runTx. If any ModuleAnteHandler fails, then runTx will return the result.

Problem Definition

It would allow users to create modules that specify their own antehandler logic and register these functions with the module manager rather than having to recreate all ante-handler logic.

If Antehandler gets refactored to seperate fee and signature logic into separate functions, then this would allow other modules to get their own ante-logic executed in the same way.

Makes it more convenient for users to define custom ante-handling logic on a per-module basis. Rather than creating a new single AnteHandler for every application that may use modules that require custom ante-handling.

Disadvantages: It is possible that this can be abused by modules who will do all message-logic checking in the antehandler. However if developers explicitly want that functionality, it is now available.

Proposal

Add AnteHandler function to AppModule interface. Add OrderAnteHandler functions to Manager interface.

Remove notion of single overseeing AnteHandler function. Instead just run all AnteHandler functions defined by ModuleManager in runTx in baseapp.go


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@AdityaSripal AdityaSripal self-assigned this Jun 17, 2019
@AdityaSripal
Copy link
Member Author

RFC @Joon, @alexanderbez

@alexanderbez
Copy link
Contributor

Looks great @AdityaSripal, thanks! I like the idea of adding an AnteHandler method to the AppModule interface. However, I don't see a reason to modify BaseApp at all. Instead, one option we can do is have SetOrderAnteHandler set the order. Then, implement AnteHandler on the Module type. This will return an AnteHandler closure by looping over all the registered module ante-handler and calling them. What are your thoughts?

e.g.

func (m *Manager) AnteHandler(ctx sdk.Context) sdk.AnteHandler {
	return func(
		ctx sdk.Context, tx sdk.Tx, simulate bool,
	) (newCtx sdk.Context, res sdk.Result, abort bool) {


		for _, moduleName := range m.OrderAnteHandler {
			newCtx, res, err := m.Modules[moduleName].AnteHalder(ctx)
			if err != nil {
				return newCtx, res, true
			}

			ctx = newCtx
		}

		return ctx, res, false
	}
}

@AdityaSripal
Copy link
Member Author

yup once i started implementing i realized its not necessary. the above is exactly how i have it

@AdityaSripal
Copy link
Member Author

Decided to go with SimpleDecorators approach in #4942

Will start implementing

@AdityaSripal AdityaSripal mentioned this issue Sep 5, 2019
5 tasks
@tnachen tnachen added this to the v0.38.0 milestone Oct 1, 2019
@alexanderbez alexanderbez mentioned this issue Oct 10, 2019
4 tasks
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

Successfully merging a pull request may close this issue.

3 participants