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.

Sign up for GitHub

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

Remove callbacks wiring in ibc go simapp #4340

Merged
merged 7 commits into from
Aug 14, 2023
Merged

Remove callbacks wiring in ibc go simapp #4340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
54fc6e2
modify all import paths to point at new SimApp
chatton Aug 14, 2023
f77dc77
added docstring for GetSimApp
chatton Aug 14, 2023
9e5fc87
fix linter
chatton Aug 14, 2023
d0ddf5a
copy contract keeper into callbacks directory
chatton Aug 14, 2023
7f3b94f
create named setup function
chatton Aug 14, 2023
ca4c004
remove dependency on callbacks module from ibc-go simapp
chatton Aug 14, 2023
d36f5e6
Merge branch 'main' into cian/issue#4339-remove-callbacks-wiring-in-i…
chatton Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions modules/apps/callbacks/ibc_middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
ibccallbacks "github.com/cosmos/ibc-go/v7/modules/apps/callbacks"
"github.com/cosmos/ibc-go/v7/modules/apps/callbacks/testing/simapp"
"github.com/cosmos/ibc-go/v7/modules/apps/callbacks/types"
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
Expand All @@ -30,14 +31,14 @@ func (s *CallbacksTestSuite) TestNewIBCMiddleware() {
{
"success",
func() {
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, channelkeeper.Keeper{}, ibcmock.ContractKeeper{}, maxCallbackGas)
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, channelkeeper.Keeper{}, simapp.ContractKeeper{}, maxCallbackGas)
},
nil,
},
{
"panics with nil underlying app",
func() {
_ = ibccallbacks.NewIBCMiddleware(nil, channelkeeper.Keeper{}, ibcmock.ContractKeeper{}, maxCallbackGas)
_ = ibccallbacks.NewIBCMiddleware(nil, channelkeeper.Keeper{}, simapp.ContractKeeper{}, maxCallbackGas)
},
fmt.Errorf("underlying application does not implement %T", (*types.CallbacksCompatibleModule)(nil)),
},
Expand All @@ -51,14 +52,14 @@ func (s *CallbacksTestSuite) TestNewIBCMiddleware() {
{
"panics with nil ics4Wrapper",
func() {
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, nil, ibcmock.ContractKeeper{}, maxCallbackGas)
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, nil, simapp.ContractKeeper{}, maxCallbackGas)
},
fmt.Errorf("ICS4Wrapper cannot be nil"),
},
{
"panics with zero maxCallbackGas",
func() {
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, channelkeeper.Keeper{}, ibcmock.ContractKeeper{}, uint64(0))
_ = ibccallbacks.NewIBCMiddleware(ibcmock.IBCModule{}, channelkeeper.Keeper{}, simapp.ContractKeeper{}, uint64(0))
},
fmt.Errorf("maxCallbackGas cannot be zero"),
},
Expand Down Expand Up @@ -128,7 +129,7 @@ func (s *CallbacksTestSuite) TestSendPacket() {
{
"failure: callback execution fails, sender is not callback address",
func() {
packetData.Sender = ibcmock.MockCallbackUnauthorizedAddress
packetData.Sender = simapp.MockCallbackUnauthorizedAddress
},
types.CallbackTypeSendPacket,
false,
Expand Down Expand Up @@ -257,7 +258,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() {
{
"failure: callback execution fails, unauthorized address",
func() {
packetData.Sender = ibcmock.MockCallbackUnauthorizedAddress
packetData.Sender = simapp.MockCallbackUnauthorizedAddress
packet.Data = packetData.GetBytes()
},
callbackFailed,
Expand Down Expand Up @@ -407,7 +408,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
{
"failure: callback execution fails, unauthorized address",
func() {
packetData.Sender = ibcmock.MockCallbackUnauthorizedAddress
packetData.Sender = simapp.MockCallbackUnauthorizedAddress
packet.Data = packetData.GetBytes()
},
callbackFailed,
Expand Down
154 changes: 0 additions & 154 deletions testing/mock/contract_keeper.go
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions testing/mock/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ const (
)

var (
StatefulCounterKey = "stateful-callback-counter"

MockAcknowledgement = channeltypes.NewResultAcknowledgement([]byte("mock acknowledgement"))
MockFailAcknowledgement = channeltypes.NewErrorAcknowledgement(fmt.Errorf("mock failed acknowledgement"))
MockPacketData = []byte("mock packet data")
Expand All @@ -43,7 +41,6 @@ var (
MockRecvCanaryCapabilityName = "mock receive canary capability name"
MockAckCanaryCapabilityName = "mock acknowledgement canary capability name"
MockTimeoutCanaryCapabilityName = "mock timeout canary capability name"
MockCallbackUnauthorizedAddress = "cosmos15ulrf36d4wdtrtqzkgaan9ylwuhs7k7qz753uk"
// MockApplicationCallbackError should be returned when an application callback should fail. It is possible to
// test that this error was returned using ErrorIs.
MockApplicationCallbackError error = &applicationCallbackError{}
Expand Down
28 changes: 5 additions & 23 deletions testing/simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ import (
ibcfee "github.com/cosmos/ibc-go/v7/modules/apps/29-fee"
ibcfeekeeper "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/keeper"
ibcfeetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types"
ibccallbacks "github.com/cosmos/ibc-go/v7/modules/apps/callbacks"
transfer "github.com/cosmos/ibc-go/v7/modules/apps/transfer"
ibctransferkeeper "github.com/cosmos/ibc-go/v7/modules/apps/transfer/keeper"
ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
Expand Down Expand Up @@ -248,9 +247,6 @@ type SimApp struct {
ScopedIBCMockKeeper capabilitykeeper.ScopedKeeper
ScopedICAMockKeeper capabilitykeeper.ScopedKeeper

// mock contract keeper used for testing
MockContractKeeper ibcmock.ContractKeeper

// make IBC modules public for test purposes
// these modules are never directly routed to by the IBC Router
ICAAuthModule ibcmock.IBCModule
Expand Down Expand Up @@ -439,10 +435,6 @@ func NewSimApp(
appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// NOTE: The mock ContractKeeper is only created for testing.
// Real applications should not use the mock ContractKeeper
app.MockContractKeeper = ibcmock.NewContractKeeper(memKeys[ibcmock.MemStoreKey])

// register the proposal types
govRouter := govv1beta1.NewRouter()
govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler).
Expand Down Expand Up @@ -499,11 +491,9 @@ func NewSimApp(
ibcRouter := porttypes.NewRouter()

// Middleware Stacks
maxCallbackGas := uint64(1_000_000)

// Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper
// since fee middleware will wrap the IBCKeeper for underlying application.
// NOTE: the Transfer Keeper's ICS4Wrapper can later be replaced.
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
Expand All @@ -525,43 +515,36 @@ func NewSimApp(

// Create Transfer Stack
// SendPacket, since it is originating from the application to core IBC:
// transferKeeper.SendPacket -> fee.SendPacket -> callbacks.SendPacket -> channel.SendPacket
// transferKeeper.SendPacket -> fee.SendPacket -> channel.SendPacket

// RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way
// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket
// channel.RecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket

// transfer stack contains (from top to bottom):
// - IBC Callbacks Middleware
// - IBC Fee Middleware
// - Transfer

// create IBC module from bottom to top of stack
var transferStack porttypes.IBCModule
transferStack = transfer.NewIBCModule(app.TransferKeeper)
transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)
transferStack = ibccallbacks.NewIBCMiddleware(transferStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
// Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the transfer keeper
app.TransferKeeper.WithICS4Wrapper(transferStack.(porttypes.Middleware))

// Add transfer stack to IBC Router
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)

// Create Interchain Accounts Stack
// SendPacket, since it is originating from the application to core IBC:
// icaAuthModuleKeeper.SendTx -> icaController.SendPacket -> fee.SendPacket -> callbacks.SendPacket -> channel.SendPacket
// icaAuthModuleKeeper.SendTx -> icaController.SendPacket -> fee.SendPacket -> channel.SendPacket

// initialize ICA module with mock module as the authentication module on the controller side
var icaControllerStack porttypes.IBCModule
icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule)
icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
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))

// RecvPacket, message that originates from core IBC and goes down to app, the flow is:
// channel.RecvPacket -> callbacks.OnRecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
// channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket

var icaHostStack porttypes.IBCModule
icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper)
Expand Down Expand Up @@ -589,8 +572,7 @@ func NewSimApp(
// create fee wrapped mock module
feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp(MockFeePort, scopedFeeMockKeeper))
app.FeeMockModule = feeMockModule
var feeWithMockModule porttypes.Middleware = ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper)
feeWithMockModule = ibccallbacks.NewIBCMiddleware(feeWithMockModule, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper)
ibcRouter.AddRoute(MockFeePort, feeWithMockModule)

// Seal the IBC Router
Expand Down