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

refactor: ibc port router for app callbacks #6314

Closed
wants to merge 51 commits into from
Closed
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
845e45f
feat: add intial SendPacket msgs and rpc definition to ibc core
damiannolan May 15, 2024
db5cddf
feat: add OnSendPacket callback to IBCModule interface
damiannolan May 15, 2024
385674a
chore: invoke OnSendPacket callback from msg server rpc
damiannolan May 15, 2024
5c861f4
chore: add comment
damiannolan May 15, 2024
9d4d2ea
chore: rename proto fields
damiannolan May 15, 2024
90fe8ae
chore: add OnSendPacket to ibccallbacks
damiannolan May 15, 2024
fc309f6
feat: add router_v2 while leaving current router impl in place
damiannolan May 15, 2024
d0f85db
chore: add getter and setters for AppRouter to core ibc
damiannolan May 15, 2024
588ff66
imp: implement callbacks OnSendPacket
colin-axner May 15, 2024
8bc82f6
imp: implement OnSendPacket for ics27
colin-axner May 16, 2024
f883d68
imp: implement OnSendPacket in transfer
damiannolan May 16, 2024
c5eef93
Merge branch 'main' into feat/port-router
damiannolan May 16, 2024
e626c7b
chore: start wiring up router_v2 in app.go
damiannolan May 16, 2024
45896aa
imp: add msg server handler to transfer
colin-axner May 16, 2024
20326d4
Merge branch 'feat/port-router' of github.com:cosmos/ibc-go into feat…
colin-axner May 16, 2024
3714960
refactor: enable core IBC SendPacket API and switch transfer to redir…
colin-axner May 16, 2024
b2df2f3
refactor: rewire controller to use SendPacket API
colin-axner May 16, 2024
b295949
refactor: rewire 29-fee to use SendPacket API
colin-axner May 16, 2024
2f5116e
refactor: use SendPacket API for callbacks
colin-axner May 16, 2024
16a88b4
imp: remove capability from SendPacket and add prefix routing
damiannolan May 16, 2024
67e4b5f
refactor: adapt signer to sdk.AccAddress
damiannolan May 16, 2024
3db1613
chore: rm capabilities from recv, ack, timeout packet handler
damiannolan May 16, 2024
07e8b1c
lint: make lint-fix and address
damiannolan May 16, 2024
4214201
refactor: rm SendPacket from ics4wrapper
damiannolan May 16, 2024
9610871
chore: rm callbacks var in SendPacket handler
damiannolan May 16, 2024
ef6e063
Merge branch 'main' into feat/port-router
damiannolan May 22, 2024
cf339f2
fix: rm capability arg in ibc ante handler
damiannolan May 22, 2024
4f8d319
Merge branch 'main' into feat/port-router
damiannolan Jul 11, 2024
a4a7bdd
(chore): remove capabilities from channel handsake (#7009)
bznein Aug 5, 2024
832a0fc
Adding updates to composite router and scaffolding for LegacyIBCModul…
chatton Aug 5, 2024
d701aa2
Rename IBCModule to ClassicIBCModule (#7015)
chatton Aug 5, 2024
9565e36
Merge branch 'main' of github.com:cosmos/ibc-go into feat/port-router
colin-axner Aug 6, 2024
3eaf2ce
fix: tests
colin-axner Aug 6, 2024
41c6656
Add Wrap/Unwrap Interfaces and implement OnChanOpenInit (#7059)
chatton Aug 7, 2024
9460400
Use new port router for onchanupgradetry (#7067)
chatton Aug 7, 2024
6ec1efa
Use new port router for OnChanOpenAck (#7084)
colin-axner Aug 7, 2024
dc6e072
Use new port router for OnChanUpgradeInit (#7082)
chatton Aug 8, 2024
ae43f7a
refactor: implement OnChanOpenConfirm using new port router (#7088)
colin-axner Aug 8, 2024
63dd289
chore: implement OnChanUpgradeTry (#7092)
chatton Aug 8, 2024
0c47e45
chore(api)!: move checks from Transfer to OnSendPacket (#7068)
bznein Aug 8, 2024
467819d
refactor: implement OnChanCloseInit using new port router. (#7095)
bznein Aug 8, 2024
d779697
Use new port router for OnChanUpgradeAck (#7094)
chatton Aug 8, 2024
179025d
refactor: implement onchancloseconfirm using new port router (#7096)
bznein Aug 8, 2024
62db721
Use new port router for OnChanUpgradeOpen (#7102)
chatton Aug 8, 2024
48b6a3f
refactor: replace ack interface with result type for OnRecvPacket (#7…
damiannolan Aug 8, 2024
d9c4e96
imp (api)!: convert coins to token only once in MsgTransfer (#7110)
bznein Aug 8, 2024
0ea612a
use new port router with OnAcknowledgementPacket (#7108)
colin-axner Aug 8, 2024
a9d34ba
chore: docs (#7111)
colin-axner Aug 12, 2024
ec593cb
Bznein/7023/on timeout packet (#7144)
bznein Aug 13, 2024
f5ab15b
Add convenience func to reverse callbacks and refactor other methods …
chatton Aug 13, 2024
6327b58
chore: use app router in OnTimeoutPacket handler (#7164)
chatton Aug 14, 2024
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
80 changes: 54 additions & 26 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/keeper"
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibcexported "github.com/cosmos/ibc-go/v9/modules/core/exported"
)
Expand All @@ -28,7 +26,7 @@ var (
// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
// ICA controller keeper and the underlying application.
type IBCMiddleware struct {
app porttypes.IBCModule
app porttypes.ClassicIBCModule
keeper keeper.Keeper
}

Expand All @@ -43,7 +41,7 @@ func NewIBCMiddleware(k keeper.Keeper) IBCMiddleware {
}

// NewIBCMiddlewareWithAuth creates a new IBCMiddleware given the associated keeper and underlying application
func NewIBCMiddlewareWithAuth(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware {
func NewIBCMiddlewareWithAuth(app porttypes.ClassicIBCModule, k keeper.Keeper) IBCMiddleware {
return IBCMiddleware{
app: app,
keeper: k,
Expand All @@ -62,28 +60,23 @@ func (im IBCMiddleware) OnChanOpenInit(
connectionHops []string,
portID string,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) (string, error) {
if !im.keeper.GetParams(ctx).ControllerEnabled {
return "", types.ErrControllerSubModuleDisabled
}

version, err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version)
version, err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, version)
if err != nil {
return "", err
}

if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
}

// call underlying app's OnChanOpenInit callback with the passed in version
// the version returned is discarded as the ica-auth module does not have permission to edit the version string.
// ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call.
if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionHops[0]) {
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, nil, counterparty, version); err != nil {
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, version); err != nil {
return "", err
}
}
Expand All @@ -98,7 +91,6 @@ func (IBCMiddleware) OnChanOpenTry(
connectionHops []string,
portID,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
counterpartyVersion string,
) (string, error) {
Expand Down Expand Up @@ -180,6 +172,56 @@ func (im IBCMiddleware) OnChanCloseConfirm(
return nil
}

// OnSendPacket implements the IBCModule interface.
func (im IBCMiddleware) OnSendPacket(
ctx sdk.Context,
portID string,
channelID string,
sequence uint64,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
signer sdk.AccAddress,
) error {
if !im.keeper.GetParams(ctx).ControllerEnabled {
return types.ErrControllerSubModuleDisabled
}

controllerPortID, err := icatypes.NewControllerPortID(signer.String())
if err != nil {
return err
}

if controllerPortID != portID {
return errorsmod.Wrap(ibcerrors.ErrUnauthorized, "signer is not owner of interchain account channel")
}

connectionID, err := im.keeper.GetConnectionID(ctx, portID, channelID)
if err != nil {
return err
}

activeChannelID, found := im.keeper.GetOpenActiveChannel(ctx, connectionID, portID)
if !found {
return errorsmod.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel on connection %s for port %s", connectionID, portID)
}

if activeChannelID != channelID {
return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "active channel ID does not match provided channelID. expected %s, got %s", activeChannelID, channelID)
}

var icaPacketData icatypes.InterchainAccountPacketData
if err := icaPacketData.UnmarshalJSON(data); err != nil {
return err
}

if err := icaPacketData.ValidateBasic(); err != nil {
return errorsmod.Wrap(err, "invalid interchain account packet data")
}

return nil
}

// OnRecvPacket implements the IBCMiddleware interface
func (IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
Expand Down Expand Up @@ -322,23 +364,9 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str
}
}

// SendPacket implements the ICS4 Wrapper interface
func (IBCMiddleware) SendPacket(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (uint64, error) {
panic(errors.New("SendPacket not supported for ICA controller module. Please use SendTx"))
}

// WriteAcknowledgement implements the ICS4 Wrapper interface
func (IBCMiddleware) WriteAcknowledgement(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
packet ibcexported.PacketI,
ack ibcexported.Acknowledgement,
) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller"
controllerkeeper "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/keeper"
"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/types"
Expand Down Expand Up @@ -130,13 +129,9 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
{
"ICA auth module does not claim channel capability", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
portID, channelID string,
counterparty channeltypes.Counterparty, version string,
) (string, error) {
if chanCap != nil {
return "", fmt.Errorf("channel capability should be nil")
}

return version, nil
}
}, true,
Expand All @@ -146,7 +141,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
// NOTE: explicitly modify the channel version via the auth module callback,
// ensuring the expected JSON encoded metadata is not modified upon return
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
portID, channelID string,
counterparty channeltypes.Counterparty, version string,
) (string, error) {
return "invalid-version", nil
Expand All @@ -161,7 +156,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
{
"ICA auth module callback fails", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
portID, channelID string,
counterparty channeltypes.Counterparty, version string,
) (string, error) {
return "", fmt.Errorf("mock ica auth fails")
Expand All @@ -178,7 +173,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)

suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
portID, channelID string,
counterparty channeltypes.Counterparty, version string,
) (string, error) {
return "", fmt.Errorf("error should be unreachable")
Expand Down Expand Up @@ -228,9 +223,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
suite.Require().NoError(err)

chanCap, err := suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
suite.Require().True(ok)

Expand All @@ -239,7 +231,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
}

version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops,
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version,
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel.Counterparty, channel.Version,
)

if tc.expPass {
Expand Down Expand Up @@ -293,12 +285,10 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenTry() {
suite.Require().True(ok)

counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
chanCap, found := suite.chainA.App.GetScopedIBCKeeper().GetCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().True(found)

version, err := cbs.OnChanOpenTry(
suite.chainA.GetContext(), path.EndpointA.ChannelConfig.Order, []string{path.EndpointA.ConnectionID},
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap,
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
counterparty, path.EndpointB.ChannelConfig.Version,
)
suite.Require().Error(err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
connectiontypes "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
Expand All @@ -26,7 +25,6 @@ func (k Keeper) OnChanOpenInit(
connectionHops []string,
portID string,
channelID string,
chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) (string, error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package keeper_test

import (
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
connectiontypes "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
Expand All @@ -19,7 +18,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
var (
channel *channeltypes.Channel
path *ibctesting.Path
chanCap *capabilitytypes.Capability
metadata icatypes.Metadata
expectedVersion string
)
Expand Down Expand Up @@ -294,13 +292,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
Version: string(versionBytes),
}

chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().NoError(err)

tc.malleate() // malleate mutates test data

version, err := suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops,
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version,
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel.Counterparty, channel.Version,
)

expPass := tc.expError == nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,80 +5,8 @@ import (

icacontrollerkeeper "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/keeper"
icacontrollertypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

func (suite *KeeperTestSuite) TestAssertChannelCapabilityMigrations() {
testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success",
func() {},
true,
},
{
"channel with different port is filtered out",
func() {
portIDWithOutPrefix := ibctesting.MockPort
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), portIDWithOutPrefix, ibctesting.FirstChannelID, channeltypes.Channel{
ConnectionHops: []string{ibctesting.FirstConnectionID},
})
},
true,
},
{
"capability not found",
func() {
portIDWithPrefix := fmt.Sprintf("%s%s", icatypes.ControllerPortPrefix, "port-without-capability")
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), portIDWithPrefix, ibctesting.FirstChannelID, channeltypes.Channel{
ConnectionHops: []string{ibctesting.FirstConnectionID},
})
},
false,
},
}

for _, ordering := range []channeltypes.Order{channeltypes.UNORDERED, channeltypes.ORDERED} {
for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

path := NewICAPath(suite.chainA, suite.chainB, ordering)
path.SetupConnections()

err := SetupICAPath(path, ibctesting.TestAccAddress)
suite.Require().NoError(err)

tc.malleate()

migrator := icacontrollerkeeper.NewMigrator(&suite.chainA.GetSimApp().ICAControllerKeeper)
err = migrator.AssertChannelCapabilityMigrations(suite.chainA.GetContext())

if tc.expPass {
suite.Require().NoError(err)

isMiddlewareEnabled := suite.chainA.GetSimApp().ICAControllerKeeper.IsMiddlewareEnabled(
suite.chainA.GetContext(),
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ConnectionID,
)

suite.Require().True(isMiddlewareEnabled)
} else {
suite.Require().Error(err)
}
})
}
}
}

func (suite *KeeperTestSuite) TestMigratorMigrateParams() {
testCases := []struct {
msg string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (s msgServer) SendTx(goCtx context.Context, msg *types.MsgSendTx) (*types.M
// the absolute timeout value is calculated using the controller chain block time + the relative timeout value
// this assumes time synchrony to a certain degree between the controller and counterparty host chain
absoluteTimeout := uint64(ctx.BlockTime().UnixNano()) + msg.RelativeTimeout
seq, err := s.sendTx(ctx, msg.ConnectionId, portID, msg.PacketData, absoluteTimeout)
seq, err := s.sendTx(ctx, msg.ConnectionId, portID, msg.PacketData, absoluteTimeout, msg.Owner)
if err != nil {
return nil, err
}
Expand Down
Loading
Loading