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

feat: add channel version to core app callbacks #6902

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func (im IBCMiddleware) OnChanCloseConfirm(
// OnRecvPacket implements the IBCMiddleware interface
func (IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
_ string,
packet channeltypes.Packet,
_ sdk.AccAddress,
) ibcexported.Acknowledgement {
Expand All @@ -195,6 +196,7 @@ func (IBCMiddleware) OnRecvPacket(
// OnAcknowledgementPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
Expand All @@ -210,7 +212,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(

// call underlying app's OnAcknowledgementPacket callback.
if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, packet.GetSourcePort(), connectionID) {
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
}

return nil
Expand All @@ -219,6 +221,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
// OnTimeoutPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnTimeoutPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
Expand All @@ -236,7 +239,7 @@ func (im IBCMiddleware) OnTimeoutPacket(
}

if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, packet.GetSourcePort(), connectionID) {
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer)
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
)

ctx := suite.chainA.GetContext()
ack := cbs.OnRecvPacket(ctx, packet, nil)
ack := cbs.OnRecvPacket(ctx, path.EndpointA.GetChannel().Version, packet, nil)
suite.Require().Equal(tc.expPass, ack.Success())

expectedEvents := sdk.Events{
Expand Down Expand Up @@ -621,7 +621,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
{
"ICA auth module callback fails", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string,
) error {
return fmt.Errorf("mock ica auth fails")
}
Expand All @@ -637,7 +637,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)

suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnAcknowledgementPacket = func(
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress,
ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string,
) error {
return fmt.Errorf("error should be unreachable")
}
Expand Down Expand Up @@ -682,7 +682,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, []byte("ack"), nil)
err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, []byte("ack"), nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -718,7 +718,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
{
"ICA auth module callback fails", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnTimeoutPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string,
) error {
return fmt.Errorf("mock ica auth fails")
}
Expand All @@ -734,7 +734,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)

suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnTimeoutPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string,
) error {
return fmt.Errorf("error should be unreachable")
}
Expand Down Expand Up @@ -779,7 +779,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)
err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
3 changes: 3 additions & 0 deletions modules/apps/27-interchain-accounts/host/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (im IBCModule) OnChanCloseConfirm(
// OnRecvPacket implements the IBCModule interface
func (im IBCModule) OnRecvPacket(
ctx sdk.Context,
_ string,
packet channeltypes.Packet,
_ sdk.AccAddress,
) ibcexported.Acknowledgement {
Expand Down Expand Up @@ -141,6 +142,7 @@ func (im IBCModule) OnRecvPacket(
// OnAcknowledgementPacket implements the IBCModule interface
func (IBCModule) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
Expand All @@ -151,6 +153,7 @@ func (IBCModule) OnAcknowledgementPacket(
// OnTimeoutPacket implements the IBCModule interface
func (IBCModule) OnTimeoutPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
Expand Down
8 changes: 4 additions & 4 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
{
"success with ICA auth module callback failure", func() {
suite.chainB.GetSimApp().ICAAuthModule.IBCApp.OnRecvPacket = func(
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress,
ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string,
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved
) exported.Acknowledgement {
return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed OnRecvPacket mock callback"))
}
Expand Down Expand Up @@ -503,7 +503,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
suite.Require().True(ok)

ctx := suite.chainB.GetContext()
ack := cbs.OnRecvPacket(ctx, packet, nil)
ack := cbs.OnRecvPacket(ctx, path.EndpointB.GetChannel().Version, packet, nil)

expectedAttributes := []sdk.Attribute{
sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName),
Expand Down Expand Up @@ -587,7 +587,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
0,
)

err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), packet, []byte("ackBytes"), nil)
err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), path.EndpointB.GetChannel().Version, packet, []byte("ackBytes"), nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -642,7 +642,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
0,
)

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)
err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
50 changes: 40 additions & 10 deletions modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,19 @@ func (im IBCMiddleware) OnChanCloseConfirm(
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {
if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) {
return im.app.OnRecvPacket(ctx, packet, relayer)
return im.app.OnRecvPacket(ctx, channelVersion, packet, relayer)
}

ack := im.app.OnRecvPacket(ctx, packet, relayer)
appVersion, err := unwrapAppVersion(channelVersion)
if err != nil {
return channeltypes.NewErrorAcknowledgement(err)
}
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
ack := im.app.OnRecvPacket(ctx, appVersion, packet, relayer)

// in case of async acknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement
if ack == nil {
Expand All @@ -241,12 +246,18 @@ func (im IBCMiddleware) OnRecvPacket(
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) {
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
}

appVersion, err := unwrapAppVersion(channelVersion)
if err != nil {
return err
}

colin-axner marked this conversation as resolved.
Show resolved Hide resolved
var ack types.IncentivizedAcknowledgement
Expand All @@ -263,14 +274,14 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
// for fee enabled channels
//
// Please see ADR 004 for more information.
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

packetID := channeltypes.NewPacketID(packet.SourcePort, packet.SourceChannel, packet.Sequence)
feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
if !found {
// call underlying callback
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel)
Expand All @@ -286,30 +297,40 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
im.keeper.DistributePacketFeesOnAcknowledgement(ctx, ack.ForwardRelayerAddress, payeeAddr, feesInEscrow.PacketFees, packetID)

// call underlying callback
return im.app.OnAcknowledgementPacket(ctx, packet, ack.AppAcknowledgement, relayer)
return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer)
}

// OnTimeoutPacket implements the IBCMiddleware interface
// If fees are not enabled, this callback will default to the ibc-core packet callback
func (im IBCMiddleware) OnTimeoutPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) {
return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer)
}

appVersion, err := unwrapAppVersion(channelVersion)
if err != nil {
return err
}

// if the fee keeper is locked then fee logic should be skipped
// this may occur in the presence of a severe bug which leads to invalid state
// the fee keeper will be unlocked after manual intervention
//
// Please see ADR 004 for more information.
if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) || im.keeper.IsLocked(ctx) {
return im.app.OnTimeoutPacket(ctx, packet, relayer)
if im.keeper.IsLocked(ctx) {
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

colin-axner marked this conversation as resolved.
Show resolved Hide resolved
packetID := channeltypes.NewPacketID(packet.SourcePort, packet.SourceChannel, packet.Sequence)
feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
if !found {
// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel)
Expand All @@ -325,7 +346,7 @@ func (im IBCMiddleware) OnTimeoutPacket(
im.keeper.DistributePacketFeesOnTimeout(ctx, payeeAddr, feesInEscrow.PacketFees, packetID)

// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer)
}

// OnChanUpgradeInit implements the IBCModule interface
Expand Down Expand Up @@ -480,3 +501,12 @@ func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID, channelID s

return unmarshaler.UnmarshalPacketData(ctx, portID, channelID, bz)
}

func unwrapAppVersion(channelVersion string) (string, error) {
metadata, err := types.MetadataFromVersion(channelVersion)
if err != nil {
return "", err
}

return metadata.AppVersion, nil
}
11 changes: 6 additions & 5 deletions modules/apps/29-fee/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
channelVersion string,
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved
) exported.Acknowledgement {
return nil
}
Expand Down Expand Up @@ -565,7 +566,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
// malleate test case
tc.malleate()

result := cbs.OnRecvPacket(suite.chainB.GetContext(), packet, suite.chainA.SenderAccount.GetAddress())
result := cbs.OnRecvPacket(suite.chainB.GetContext(), suite.path.EndpointB.GetChannel().Version, packet, suite.chainA.SenderAccount.GetAddress())

switch {
case tc.name == "success":
Expand Down Expand Up @@ -798,7 +799,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
{
"application callback fails",
func() {
suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnAcknowledgementPacket = func(_ sdk.Context, _ channeltypes.Packet, _ []byte, _ sdk.AccAddress) error {
suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnAcknowledgementPacket = func(_ sdk.Context, _ channeltypes.Packet, _ []byte, _ sdk.AccAddress, _ string) error {
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("mock fee app callback fails")
}
},
Expand Down Expand Up @@ -839,7 +840,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
suite.Require().True(ok)

err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, ack, relayerAddr)
err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, ack, relayerAddr)

if tc.expPass {
suite.Require().NoError(err)
Expand Down Expand Up @@ -1012,7 +1013,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
{
"application callback fails",
func() {
suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnTimeoutPacket = func(_ sdk.Context, _ channeltypes.Packet, _ sdk.AccAddress) error {
suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnTimeoutPacket = func(_ sdk.Context, _ channeltypes.Packet, _ sdk.AccAddress, _ string) error {
gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("mock fee app callback fails")
}
},
Expand Down Expand Up @@ -1050,7 +1051,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
suite.Require().True(ok)

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, relayerAddr)
err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, relayerAddr)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
11 changes: 6 additions & 5 deletions modules/apps/callbacks/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,13 @@ func (im IBCMiddleware) SendPacket(
// reverted via a panic.
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
channelVersion string,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
// we first call the underlying app to handle the acknowledgement
err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
err := im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer)
if err != nil {
return err
}
Expand Down Expand Up @@ -168,8 +169,8 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
// It defers to the underlying application and then calls the contract callback.
// If the contract callback runs out of gas and may be retried with a higher gas limit then the state changes are
// reverted via a panic.
func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error {
err := im.app.OnTimeoutPacket(ctx, packet, relayer)
func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error {
err := im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer)
if err != nil {
return err
}
Expand Down Expand Up @@ -201,8 +202,8 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac
// It defers to the underlying application and then calls the contract callback.
// If the contract callback runs out of gas and may be retried with a higher gas limit then the state changes are
// reverted via a panic.
func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement {
ack := im.app.OnRecvPacket(ctx, packet, relayer)
func (im IBCMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) ibcexported.Acknowledgement {
ack := im.app.OnRecvPacket(ctx, channelVersion, packet, relayer)
// if ack is nil (asynchronous acknowledgements), then the callback will be handled in WriteAcknowledgement
// if ack is not successful, all state changes are reverted. If a packet cannot be received, then there is
// no need to execute a callback on the receiving chain.
Expand Down
Loading
Loading