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

Move callbacks middleware down the middleware stack #4444

Open
6 tasks
srdtrk opened this issue Aug 24, 2023 · 1 comment
Open
6 tasks

Move callbacks middleware down the middleware stack #4444

srdtrk opened this issue Aug 24, 2023 · 1 comment
Labels
callbacks middleware Issues for callbacks middleware type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@srdtrk
Copy link
Member

srdtrk commented Aug 24, 2023

Summary

Wire up the Callbacks Middleware so that it is the middle app in the middleware stack.

Problem Definition

See this thread for a detailed explanation. The main issue with the current setup is that it wouldn't have worked if callbacks had a version.

  • Change the wiring of the transfer stack in callbacks/simapp
  • Change the wiring of the icacontroller stack in callbacks/simapp
  • Update the callbacks middleware documentation integration.md

Proposal

For example, the ICA controller stack would look like:

    var icaControllerStack porttypes.IBCModule
    icaControllerStack = icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
    icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
    // Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
    app.ICAControllerKeeper.WithICS4Wrapper(icaControllerStack.(porttypes.Middleware))
    
    icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@srdtrk srdtrk added type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. callbacks middleware Issues for callbacks middleware labels Aug 24, 2023
@srdtrk
Copy link
Member Author

srdtrk commented Aug 31, 2023

Rewiring transfer stack this way causes some unit tests in ibc_middleware_test.go to fail. Since the middleware is initialised in the middle of the stack. There would be no way to unit test its SendPacket without also testing other functions. I think we should address this after #4329 and #4328

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callbacks middleware Issues for callbacks middleware type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants