cosmos / ibc-go

Inter-Blockchain Communication Protocol (IBC) implementation in Golang.
https://ibc.cosmos.network/
MIT License
524 stars 565 forks source link

Move callbacks middleware down the middleware stack #4444

Open srdtrk opened 10 months ago

srdtrk commented 10 months ago

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.

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

srdtrk commented 10 months ago

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 https://github.com/cosmos/ibc-go/issues/4329 and https://github.com/cosmos/ibc-go/issues/4328