From 018ae6f2c349d1a73262bf2d1b76f451b6fae734 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Fri, 19 Jul 2024 12:53:35 +0200 Subject: [PATCH 1/5] feat: add channel version to core app callbacks --- .../controller/ibc_middleware.go | 7 ++- .../controller/ibc_middleware_test.go | 14 ++--- .../27-interchain-accounts/host/ibc_module.go | 3 + .../host/ibc_module_test.go | 8 +-- modules/apps/29-fee/ibc_middleware.go | 21 ++++--- modules/apps/29-fee/ibc_middleware_test.go | 11 ++-- modules/apps/callbacks/ibc_middleware.go | 11 ++-- modules/apps/callbacks/ibc_middleware_test.go | 8 +-- modules/apps/transfer/ibc_module.go | 3 + modules/apps/transfer/ibc_module_test.go | 6 +- .../transfer/keeper/relay_forwarding_test.go | 2 +- modules/core/04-channel/keeper/packet.go | 56 +++++++++---------- modules/core/04-channel/keeper/packet_test.go | 39 ++++++++----- modules/core/04-channel/keeper/timeout.go | 48 ++++++++-------- .../core/04-channel/keeper/timeout_test.go | 8 ++- modules/core/05-port/types/module.go | 3 + modules/core/ante/ante.go | 2 +- modules/core/ante/ante_test.go | 4 +- modules/core/keeper/msg_server.go | 16 +++--- testing/mock/ibc_app.go | 3 + testing/mock/ibc_module.go | 12 ++-- testing/mock/middleware.go | 12 ++-- 22 files changed, 165 insertions(+), 132 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index f6f14d894bf..b9192b28029 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -185,6 +185,7 @@ func (IBCMiddleware) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, _ sdk.AccAddress, + _ string, ) ibcexported.Acknowledgement { err := errorsmod.Wrapf(icatypes.ErrInvalidChannelFlow, "cannot receive packet on controller chain") ack := channeltypes.NewErrorAcknowledgement(err) @@ -198,6 +199,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error { if !im.keeper.GetParams(ctx).ControllerEnabled { return types.ErrControllerSubModuleDisabled @@ -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, packet, acknowledgement, relayer, channelVersion) } return nil @@ -221,6 +223,7 @@ func (im IBCMiddleware) OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error { if !im.keeper.GetParams(ctx).ControllerEnabled { return types.ErrControllerSubModuleDisabled @@ -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, packet, relayer, channelVersion) } return nil diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 0028b325fd8..72dfc3b6eeb 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -577,7 +577,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { ) ctx := suite.chainA.GetContext() - ack := cbs.OnRecvPacket(ctx, packet, nil) + ack := cbs.OnRecvPacket(ctx, packet, nil, path.EndpointA.GetChannel().Version) suite.Require().Equal(tc.expPass, ack.Success()) expectedEvents := sdk.Events{ @@ -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") } @@ -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") } @@ -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(), packet, []byte("ack"), nil, path.EndpointA.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) @@ -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") } @@ -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") } @@ -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(), packet, nil, path.EndpointA.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 7e9d2eaed23..622ed24048d 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -115,6 +115,7 @@ func (im IBCModule) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, _ sdk.AccAddress, + _ string, ) ibcexported.Acknowledgement { if !im.keeper.GetParams(ctx).HostEnabled { im.keeper.Logger(ctx).Info("host submodule is disabled") @@ -144,6 +145,7 @@ func (IBCModule) OnAcknowledgementPacket( packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot receive acknowledgement on a host channel end, a host chain does not send a packet over the channel") } @@ -153,6 +155,7 @@ func (IBCModule) OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot cause a packet timeout on a host channel end, a host chain does not send a packet over the channel") } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index d13d71c3d52..6bfc07a6890 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -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, ) exported.Acknowledgement { return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed OnRecvPacket mock callback")) } @@ -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, packet, nil, path.EndpointB.GetChannel().Version) expectedAttributes := []sdk.Attribute{ sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName), @@ -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(), packet, []byte("ackBytes"), nil, path.EndpointB.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) @@ -642,7 +642,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { 0, ) - err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil, path.EndpointA.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 5239754e6dc..8e2976a6c66 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -218,12 +218,13 @@ func (im IBCMiddleware) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) exported.Acknowledgement { if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) { - return im.app.OnRecvPacket(ctx, packet, relayer) + return im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) } - ack := im.app.OnRecvPacket(ctx, packet, relayer) + ack := im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) // in case of async acknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement if ack == nil { @@ -244,9 +245,10 @@ func (im IBCMiddleware) OnAcknowledgementPacket( packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) } var ack types.IncentivizedAcknowledgement @@ -263,14 +265,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, packet, ack.AppAcknowledgement, relayer, channelVersion) } 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, packet, ack.AppAcknowledgement, relayer, channelVersion) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -286,7 +288,7 @@ 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, packet, ack.AppAcknowledgement, relayer, channelVersion) } // OnTimeoutPacket implements the IBCMiddleware interface @@ -295,6 +297,7 @@ func (im IBCMiddleware) OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error { // 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 @@ -302,14 +305,14 @@ func (im IBCMiddleware) OnTimeoutPacket( // // 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) + return im.app.OnTimeoutPacket(ctx, packet, relayer, channelVersion) } 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, packet, relayer, channelVersion) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -325,7 +328,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, packet, relayer, channelVersion) } // OnChanUpgradeInit implements the IBCModule interface diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index 4bf51bb56b2..17c4a249eb6 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -515,6 +515,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) exported.Acknowledgement { return nil } @@ -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(), packet, suite.chainA.SenderAccount.GetAddress(), suite.path.EndpointB.GetChannel().Version) switch { case tc.name == "success": @@ -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 { return fmt.Errorf("mock fee app callback fails") } }, @@ -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(), packet, ack, relayerAddr, suite.path.EndpointA.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) @@ -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 { return fmt.Errorf("mock fee app callback fails") } }, @@ -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(), packet, relayerAddr, suite.path.EndpointA.GetChannel().Version) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index ef6dbcbbfd0..7fe93920c9f 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -133,9 +133,10 @@ func (im IBCMiddleware) OnAcknowledgementPacket( packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error { // we first call the underlying app to handle the acknowledgement - err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) if err != nil { return err } @@ -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, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) error { + err := im.app.OnTimeoutPacket(ctx, packet, relayer, channelVersion) if err != nil { return err } @@ -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, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) ibcexported.Acknowledgement { + ack := im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) // 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. diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 25bc6db3785..880d10df072 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -355,7 +355,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { s.Require().True(ok) onAcknowledgementPacket := func() error { - return transferStack.OnAcknowledgementPacket(ctx, packet, ack, s.chainA.SenderAccount.GetAddress()) + return transferStack.OnAcknowledgementPacket(ctx, packet, ack, s.chainA.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) } switch tc.expError { @@ -519,7 +519,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { s.Require().True(ok) onTimeoutPacket := func() error { - return transferStack.OnTimeoutPacket(ctx, packet, s.chainA.SenderAccount.GetAddress()) + return transferStack.OnTimeoutPacket(ctx, packet, s.chainA.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) } switch expValue := tc.expValue.(type) { @@ -687,7 +687,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { s.Require().True(ok) onRecvPacket := func() ibcexported.Acknowledgement { - return transferStack.OnRecvPacket(ctx, packet, s.chainB.SenderAccount.GetAddress()) + return transferStack.OnRecvPacket(ctx, packet, s.chainB.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) } switch tc.expAck { @@ -1132,7 +1132,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacketAsyncAck() { 0, ) - ack := mockFeeCallbackStack.OnRecvPacket(s.chainA.GetContext(), packet, s.chainA.SenderAccount.GetAddress()) + ack := mockFeeCallbackStack.OnRecvPacket(s.chainA.GetContext(), packet, s.chainA.SenderAccount.GetAddress(), ibcmock.MockFeeVersion) s.Require().Nil(ack) s.AssertHasExecutedExpectedCallback("none", true) } diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index b022cae66cb..aca28aa97ed 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -183,6 +183,7 @@ func (im IBCModule) OnRecvPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) ibcexported.Acknowledgement { var ( ackErr error @@ -227,6 +228,7 @@ func (im IBCModule) OnAcknowledgementPacket( packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error { var ack channeltypes.Acknowledgement if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { @@ -252,6 +254,7 @@ func (im IBCModule) OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error { data, err := im.getICS20PacketData(ctx, packet.GetData(), packet.GetSourcePort(), packet.GetSourceChannel()) if err != nil { diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 2beb583cc5a..f3d59f93fb6 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -403,7 +403,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() { tc.malleate() // change fields in packet - ack := cbs.OnRecvPacket(ctx, packet, suite.chainB.SenderAccount.GetAddress()) + ack := cbs.OnRecvPacket(ctx, packet, suite.chainB.SenderAccount.GetAddress(), path.EndpointB.GetChannel().Version) suite.Require().Equal(tc.expAck, ack) @@ -468,7 +468,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() { cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module) suite.Require().True(ok) - suite.Require().NoError(cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress())) + suite.Require().NoError(cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress(), path.EndpointA.GetChannel().Version)) }, errors.New("unable to unescrow tokens"), }, @@ -508,7 +508,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() { tc.malleate() // change fields in packet - err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress()) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress(), path.EndpointA.GetChannel().Version) expPass := tc.expError == nil if expPass { diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index b037d14033a..dc09a12ac48 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -983,7 +983,7 @@ func (suite *ForwardingTestSuite) TestOnTimeoutPacketForwarding() { suite.Require().True(ok) // Trigger OnTimeoutPacket for chainB - err = cbs.OnTimeoutPacket(suite.chainB.GetContext(), packet, nil) + err = cbs.OnTimeoutPacket(suite.chainB.GetContext(), packet, nil, pathBtoC.EndpointA.GetChannel().Version) suite.Require().NoError(err) // Ensure that chainB has an ack. diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go index f56b578096f..04f8768a712 100644 --- a/modules/core/04-channel/keeper/packet.go +++ b/modules/core/04-channel/keeper/packet.go @@ -111,14 +111,14 @@ func (k *Keeper) RecvPacket( packet types.Packet, proof []byte, proofHeight exported.Height, -) error { +) (string, error) { channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel()) if !found { - return errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestChannel()) + return "", errorsmod.Wrap(types.ErrChannelNotFound, packet.GetDestChannel()) } if !slices.Contains([]types.State{types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE}, channel.State) { - return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of [%s, %s, %s], but got %s", types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE, channel.State) + return "", errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of [%s, %s, %s], but got %s", types.OPEN, types.FLUSHING, types.FLUSHCOMPLETE, channel.State) } // If counterpartyUpgrade is stored we need to ensure that the @@ -130,14 +130,14 @@ func (k *Keeper) RecvPacket( if found { counterpartyNextSequenceSend := counterpartyUpgrade.NextSequenceSend if packet.GetSequence() >= counterpartyNextSequenceSend { - return errorsmod.Wrapf(types.ErrInvalidPacket, "cannot flush packet at sequence greater than or equal to counterparty next sequence send (%d) ≥ (%d).", packet.GetSequence(), counterpartyNextSequenceSend) + return "", errorsmod.Wrapf(types.ErrInvalidPacket, "cannot flush packet at sequence greater than or equal to counterparty next sequence send (%d) ≥ (%d).", packet.GetSequence(), counterpartyNextSequenceSend) } } // Authenticate capability to ensure caller has authority to receive packet on this channel capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel()) if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidChannelCapability, "channel capability failed authentication for capability name %s", capName, ) @@ -145,14 +145,14 @@ func (k *Keeper) RecvPacket( // packet must come from the channel's counterparty if packet.GetSourcePort() != channel.Counterparty.PortId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet source port doesn't match the counterparty's port (%s ≠ %s)", packet.GetSourcePort(), channel.Counterparty.PortId, ) } if packet.GetSourceChannel() != channel.Counterparty.ChannelId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet source channel doesn't match the counterparty's channel (%s ≠ %s)", packet.GetSourceChannel(), channel.Counterparty.ChannelId, ) @@ -163,18 +163,18 @@ func (k *Keeper) RecvPacket( // connection and channel must both be open connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0]) if !found { - return errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) + return "", errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) } if connectionEnd.State != connectiontypes.OPEN { - return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectionEnd.State) + return "", errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectionEnd.State) } // check if packet timed out by comparing it with the latest height of the chain selfHeight, selfTimestamp := clienttypes.GetSelfHeight(ctx), uint64(ctx.BlockTime().UnixNano()) timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp()) if timeout.Elapsed(selfHeight, selfTimestamp) { - return errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed") + return "", errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed") } commitment := types.CommitPacket(k.cdc, packet) @@ -185,11 +185,11 @@ func (k *Keeper) RecvPacket( packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), commitment, ); err != nil { - return errorsmod.Wrap(err, "couldn't verify counterparty packet commitment") + return "", errorsmod.Wrap(err, "couldn't verify counterparty packet commitment") } if err := k.applyReplayProtection(ctx, packet, channel); err != nil { - return err + return "", err } // log that a packet has been received & executed @@ -205,7 +205,7 @@ func (k *Keeper) RecvPacket( // emit an event that the relayer can query for emitRecvPacketEvent(ctx, packet, channel) - return nil + return channel.Version, nil } // applyReplayProtection ensures a packet has not already been received @@ -374,23 +374,23 @@ func (k *Keeper) AcknowledgePacket( acknowledgement []byte, proof []byte, proofHeight exported.Height, -) error { +) (string, error) { channel, found := k.GetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) if !found { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", packet.GetSourcePort(), packet.GetSourceChannel(), ) } if !slices.Contains([]types.State{types.OPEN, types.FLUSHING}, channel.State) { - return errorsmod.Wrapf(types.ErrInvalidChannelState, "packets cannot be acknowledged on channel with state (%s)", channel.State) + return "", errorsmod.Wrapf(types.ErrInvalidChannelState, "packets cannot be acknowledged on channel with state (%s)", channel.State) } // Authenticate capability to ensure caller has authority to receive packet on this channel capName := host.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel()) if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidChannelCapability, "channel capability failed authentication for capability name %s", capName, ) @@ -398,14 +398,14 @@ func (k *Keeper) AcknowledgePacket( // packet must have been sent to the channel's counterparty if packet.GetDestPort() != channel.Counterparty.PortId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination port doesn't match the counterparty's port (%s ≠ %s)", packet.GetDestPort(), channel.Counterparty.PortId, ) } if packet.GetDestChannel() != channel.Counterparty.ChannelId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination channel doesn't match the counterparty's channel (%s ≠ %s)", packet.GetDestChannel(), channel.Counterparty.ChannelId, ) @@ -413,11 +413,11 @@ func (k *Keeper) AcknowledgePacket( connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0]) if !found { - return errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) + return "", errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) } if connectionEnd.State != connectiontypes.OPEN { - return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectionEnd.State) + return "", errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectionEnd.State) } commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) @@ -428,35 +428,35 @@ func (k *Keeper) AcknowledgePacket( // or there is a misconfigured relayer attempting to prove an acknowledgement // for a packet never sent. Core IBC will treat this error as a no-op in order to // prevent an entire relay transaction from failing and consuming unnecessary fees. - return types.ErrNoOpMsg + return "", types.ErrNoOpMsg } packetCommitment := types.CommitPacket(k.cdc, packet) // verify we sent the packet and haven't cleared it out yet if !bytes.Equal(commitment, packetCommitment) { - return errorsmod.Wrapf(types.ErrInvalidPacket, "commitment bytes are not equal: got (%v), expected (%v)", packetCommitment, commitment) + return "", errorsmod.Wrapf(types.ErrInvalidPacket, "commitment bytes are not equal: got (%v), expected (%v)", packetCommitment, commitment) } if err := k.connectionKeeper.VerifyPacketAcknowledgement( ctx, connectionEnd, proofHeight, proof, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence(), acknowledgement, ); err != nil { - return err + return "", err } // assert packets acknowledged in order if channel.Ordering == types.ORDERED { nextSequenceAck, found := k.GetNextSequenceAck(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) if !found { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrSequenceAckNotFound, "source port: %s, source channel: %s", packet.GetSourcePort(), packet.GetSourceChannel(), ) } if packet.GetSequence() != nextSequenceAck { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrPacketSequenceOutOfOrder, "packet sequence ≠ next ack sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceAck, ) @@ -491,11 +491,11 @@ func (k *Keeper) AcknowledgePacket( if channel.State == types.FLUSHING { if aborted := k.handleFlushState(ctx, packet, channel); aborted { k.Logger(ctx).Info("upgrade aborted", "port_id", packet.GetSourcePort(), "channel_id", packet.GetSourceChannel(), "upgrade_sequence", channel.UpgradeSequence) - return nil + return channel.Version, nil } } - return nil + return channel.Version, nil } // handleFlushState is called when a packet is acknowledged or timed out and the channel is in diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 97b6e5795bf..acd7197af19 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -666,11 +666,12 @@ func (suite *KeeperTestSuite) TestRecvPacket() { packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) proof, proofHeight := path.EndpointA.QueryProof(packetKey) - err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.RecvPacket(suite.chainB.GetContext(), channelCap, packet, proof, proofHeight) + channelVersion, err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.RecvPacket(suite.chainB.GetContext(), channelCap, packet, proof, proofHeight) expPass := tc.expError == nil if expPass { suite.Require().NoError(err) + suite.Require().Equal(path.EndpointA.GetChannel().Version, channelVersion, "channel version is incorrect") channelB, _ := suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetChannel(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel()) nextSeqRecv, found := suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceRecv(suite.chainB.GetContext(), packet.GetDestPort(), packet.GetDestChannel()) @@ -688,6 +689,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() { } else { suite.Require().Error(err) suite.Require().ErrorIs(err, tc.expError) + suite.Require().Equal("", channelVersion) } }) } @@ -838,24 +840,27 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap *capabilitytypes.Capability ) - assertErr := func(errType *errors.Error) func(commitment []byte, err error) { - return func(commitment []byte, err error) { + assertErr := func(errType *errors.Error) func(commitment []byte, channelVersion string, err error) { + return func(commitment []byte, channelVersion string, err error) { suite.Require().Error(err) suite.Require().ErrorIs(err, errType) suite.Require().NotNil(commitment) + suite.Require().Equal("", channelVersion) } } - assertNoOp := func(commitment []byte, err error) { + assertNoOp := func(commitment []byte, channelVersion string, err error) { suite.Require().Error(err) suite.Require().ErrorIs(err, types.ErrNoOpMsg) suite.Require().Nil(commitment) + suite.Require().Equal("", channelVersion) } - assertSuccess := func(seq func() uint64, msg string) func(commitment []byte, err error) { - return func(commitment []byte, err error) { + assertSuccess := func(seq func() uint64, msg string) func(commitment []byte, channelVersion string, err error) { + return func(commitment []byte, channelVersion string, err error) { suite.Require().NoError(err) suite.Require().Nil(commitment) + suite.Require().Equal(path.EndpointA.GetChannel().Version, channelVersion) nextSequenceAck, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) @@ -867,7 +872,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { testCases := []struct { name string malleate func() - expResult func(commitment []byte, err error) + expResult func(commitment []byte, channelVersion string, err error) expEvents func(path *ibctesting.Path) []abci.Event }{ { @@ -927,12 +932,13 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { path.EndpointA.UpdateChannel(func(channel *types.Channel) { channel.State = types.FLUSHING }) }, - expResult: func(commitment []byte, err error) { + expResult: func(commitment []byte, channelVersion string, err error) { suite.Require().NoError(err) suite.Require().Nil(commitment) channel := path.EndpointA.GetChannel() suite.Require().Equal(types.FLUSHING, channel.State) + suite.Require().Equal(channel.Version, channelVersion) nextSequenceAck, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) suite.Require().True(found) @@ -964,12 +970,13 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { path.EndpointA.SetChannelCounterpartyUpgrade(counterpartyUpgrade) }, - expResult: func(commitment []byte, err error) { + expResult: func(commitment []byte, channelVersion string, err error) { suite.Require().NoError(err) suite.Require().Nil(commitment) channel := path.EndpointA.GetChannel() suite.Require().Equal(types.FLUSHCOMPLETE, channel.State) + suite.Require().Equal(channel.Version, channelVersion) nextSequenceAck, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) suite.Require().True(found) @@ -1024,12 +1031,13 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { path.EndpointA.SetChannelUpgrade(upgrade) path.EndpointA.SetChannelCounterpartyUpgrade(counterpartyUpgrade) }, - expResult: func(commitment []byte, err error) { + expResult: func(commitment []byte, channelVersion string, err error) { suite.Require().NoError(err) suite.Require().Nil(commitment) channel := path.EndpointA.GetChannel() suite.Require().Equal(types.OPEN, channel.State) + suite.Require().Equal(channel.Version, channelVersion) nextSequenceAck, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel()) suite.Require().True(found) @@ -1123,10 +1131,11 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { path.EndpointA.UpdateChannel(func(channel *types.Channel) { channel.State = types.FLUSHCOMPLETE }) }, - expResult: func(commitment []byte, err error) { + expResult: func(commitment []byte, channelVersion string, err error) { suite.Require().Error(err) suite.Require().ErrorIs(err, types.ErrInvalidChannelState) suite.Require().Nil(commitment) + suite.Require().Equal("", channelVersion) }, }, { @@ -1193,7 +1202,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel( suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{"connection-1000"}, path.EndpointA.ChannelConfig.Version), + types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{"connection-1000"}, path.EndpointA.GetChannel().Version), ) suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -1220,7 +1229,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel( suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, path.EndpointA.ChannelConfig.Version), + types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, path.EndpointA.GetChannel().Version), ) suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -1334,10 +1343,10 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) proof, proofHeight := path.EndpointB.QueryProof(packetKey) - err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(ctx, channelCap, packet, ack.Acknowledgement(), proof, proofHeight) + channelVersion, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(ctx, channelCap, packet, ack.Acknowledgement(), proof, proofHeight) commitment := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPacketCommitment(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, packet.GetSequence()) - tc.expResult(commitment, err) + tc.expResult(commitment, channelVersion, err) if tc.expEvents != nil { events := ctx.EventManager().ABCIEvents() diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go index 1f8649e0f7a..36d82873707 100644 --- a/modules/core/04-channel/keeper/timeout.go +++ b/modules/core/04-channel/keeper/timeout.go @@ -28,10 +28,10 @@ func (k *Keeper) TimeoutPacket( proof []byte, proofHeight exported.Height, nextSequenceRecv uint64, -) error { +) (string, error) { channel, found := k.GetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) if !found { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", packet.GetSourcePort(), packet.GetSourceChannel(), ) @@ -41,14 +41,14 @@ func (k *Keeper) TimeoutPacket( // so the capability authentication can be omitted here if packet.GetDestPort() != channel.Counterparty.PortId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination port doesn't match the counterparty's port (%s ≠ %s)", packet.GetDestPort(), channel.Counterparty.PortId, ) } if packet.GetDestChannel() != channel.Counterparty.ChannelId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination channel doesn't match the counterparty's channel (%s ≠ %s)", packet.GetDestChannel(), channel.Counterparty.ChannelId, ) @@ -56,7 +56,7 @@ func (k *Keeper) TimeoutPacket( connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0]) if !found { - return errorsmod.Wrap( + return "", errorsmod.Wrap( connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0], ) @@ -65,12 +65,12 @@ func (k *Keeper) TimeoutPacket( // check that timeout height or timeout timestamp has passed on the other end proofTimestamp, err := k.clientKeeper.GetClientTimestampAtHeight(ctx, connectionEnd.ClientId, proofHeight) if err != nil { - return err + return "", err } timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp()) if !timeout.Elapsed(proofHeight.(clienttypes.Height), proofTimestamp) { - return errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached") + return "", errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached") } commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) @@ -81,21 +81,21 @@ func (k *Keeper) TimeoutPacket( // or there is a misconfigured relayer attempting to prove a timeout // for a packet never sent. Core IBC will treat this error as a no-op in order to // prevent an entire relay transaction from failing and consuming unnecessary fees. - return types.ErrNoOpMsg + return "", types.ErrNoOpMsg } packetCommitment := types.CommitPacket(k.cdc, packet) // verify we sent the packet and haven't cleared it out yet if !bytes.Equal(commitment, packetCommitment) { - return errorsmod.Wrapf(types.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) + return "", errorsmod.Wrapf(types.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) } switch channel.Ordering { case types.ORDERED: // check that packet has not been received if nextSequenceRecv > packet.GetSequence() { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrPacketReceived, "packet already received, next sequence receive > packet sequence (%d > %d)", nextSequenceRecv, packet.GetSequence(), ) @@ -116,11 +116,11 @@ func (k *Keeper) TimeoutPacket( } if err != nil { - return err + return "", err } // NOTE: the remaining code is located in the TimeoutExecuted function - return nil + return channel.Version, nil } // TimeoutExecuted deletes the commitment send from this chain after it verifies timeout. @@ -203,29 +203,29 @@ func (k *Keeper) TimeoutOnClose( proofHeight exported.Height, nextSequenceRecv uint64, counterpartyUpgradeSequence uint64, -) error { +) (string, error) { channel, found := k.GetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) if !found { - return errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", packet.GetSourcePort(), packet.GetSourceChannel()) + return "", errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", packet.GetSourcePort(), packet.GetSourceChannel()) } capName := host.ChannelCapabilityPath(packet.GetSourcePort(), packet.GetSourceChannel()) if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidChannelCapability, "channel capability failed authentication with capability name %s", capName, ) } if packet.GetDestPort() != channel.Counterparty.PortId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination port doesn't match the counterparty's port (%s ≠ %s)", packet.GetDestPort(), channel.Counterparty.PortId, ) } if packet.GetDestChannel() != channel.Counterparty.ChannelId { - return errorsmod.Wrapf( + return "", errorsmod.Wrapf( types.ErrInvalidPacket, "packet destination channel doesn't match the counterparty's channel (%s ≠ %s)", packet.GetDestChannel(), channel.Counterparty.ChannelId, ) @@ -233,7 +233,7 @@ func (k *Keeper) TimeoutOnClose( connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0]) if !found { - return errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) + return "", errorsmod.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0]) } commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) @@ -244,14 +244,14 @@ func (k *Keeper) TimeoutOnClose( // or there is a misconfigured relayer attempting to prove a timeout // for a packet never sent. Core IBC will treat this error as a no-op in order to // prevent an entire relay transaction from failing and consuming unnecessary fees. - return types.ErrNoOpMsg + return "", types.ErrNoOpMsg } packetCommitment := types.CommitPacket(k.cdc, packet) // verify we sent the packet and haven't cleared it out yet if !bytes.Equal(commitment, packetCommitment) { - return errorsmod.Wrapf(types.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) + return "", errorsmod.Wrapf(types.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) } counterpartyHops := []string{connectionEnd.Counterparty.ConnectionId} @@ -272,7 +272,7 @@ func (k *Keeper) TimeoutOnClose( channel.Counterparty.PortId, channel.Counterparty.ChannelId, expectedChannel, ); err != nil { - return err + return "", err } var err error @@ -280,7 +280,7 @@ func (k *Keeper) TimeoutOnClose( case types.ORDERED: // check that packet has not been received if nextSequenceRecv > packet.GetSequence() { - return errorsmod.Wrapf(types.ErrInvalidPacket, "packet already received, next sequence receive > packet sequence (%d > %d", nextSequenceRecv, packet.GetSequence()) + return "", errorsmod.Wrapf(types.ErrInvalidPacket, "packet already received, next sequence receive > packet sequence (%d > %d", nextSequenceRecv, packet.GetSequence()) } // check that the recv sequence is as claimed @@ -298,9 +298,9 @@ func (k *Keeper) TimeoutOnClose( } if err != nil { - return err + return "", err } // NOTE: the remaining code is located in the TimeoutExecuted function - return nil + return channel.Version, nil } diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go index 126bb783764..26b55158907 100644 --- a/modules/core/04-channel/keeper/timeout_test.go +++ b/modules/core/04-channel/keeper/timeout_test.go @@ -220,12 +220,14 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { } } - err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.TimeoutPacket(suite.chainA.GetContext(), packet, proof, proofHeight, nextSeqRecv) + channelVersion, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.TimeoutPacket(suite.chainA.GetContext(), packet, proof, proofHeight, nextSeqRecv) if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(path.EndpointA.GetChannel().Version, channelVersion) } else { suite.Require().Error(err) + suite.Require().Equal("", channelVersion) // only check if expError is set, since not all error codes can be known if expError != nil { suite.Require().True(errors.Is(err, expError)) @@ -757,7 +759,7 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { proof, _ = suite.chainB.QueryProof(unorderedPacketKey) } - err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.TimeoutOnClose( + channelVersion, err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.TimeoutOnClose( suite.chainA.GetContext(), chanCap, packet, @@ -770,8 +772,10 @@ func (suite *KeeperTestSuite) TestTimeoutOnClose() { if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(path.EndpointA.GetChannel().Version, channelVersion) } else { suite.Require().Error(err) + suite.Require().Equal("", channelVersion) } }) } diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 7e9b3bf672e..bfa77480dba 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -90,6 +90,7 @@ type IBCModule interface { ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) exported.Acknowledgement OnAcknowledgementPacket( @@ -97,12 +98,14 @@ type IBCModule interface { packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error OnTimeoutPacket( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error } diff --git a/modules/core/ante/ante.go b/modules/core/ante/ante.go index 9b37e0d79b4..2d08ba0db16 100644 --- a/modules/core/ante/ante.go +++ b/modules/core/ante/ante.go @@ -117,7 +117,7 @@ func (rrd RedundantRelayDecorator) recvPacketCheckTx(ctx sdk.Context, msg *chann // If the packet was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - err = rrd.k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) + _, err = rrd.k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) switch err { case nil: diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index f60b75ebd50..f852896475b 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -351,7 +351,7 @@ func (suite *AnteTestSuite) TestAnteDecoratorCheckTx() { "success on app callback error, app callbacks are skipped for performance", func(suite *AnteTestSuite) []sdk.Msg { suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func( - ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string, ) exported.Acknowledgement { panic(fmt.Errorf("failed OnRecvPacket mock callback")) } @@ -587,7 +587,7 @@ func (suite *AnteTestSuite) TestAnteDecoratorReCheckTx() { "success on app callback error, app callbacks are skipped for performance", func(suite *AnteTestSuite) []sdk.Msg { suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func( - ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string, ) exported.Acknowledgement { panic(fmt.Errorf("failed OnRecvPacket mock callback")) } diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 345f73c5dce..6bd816257ad 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -472,7 +472,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack // If the packet was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - err = k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) + channelVersion, err := k.ChannelKeeper.RecvPacket(cacheCtx, capability, msg.Packet, msg.ProofCommitment, msg.ProofHeight) switch err { case nil: @@ -490,7 +490,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack // // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. cacheCtx, writeFn = ctx.CacheContext() - ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer) + ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer, channelVersion) if ack == nil || ack.Success() { // write application state changes for asynchronous and successful acknowledgements writeFn() @@ -544,7 +544,7 @@ func (k *Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (* // If the timeout was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - err = k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv) + channelVersion, err := k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv) switch err { case nil: @@ -564,7 +564,7 @@ func (k *Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (* } // Perform application logic callback - err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer) + err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer, channelVersion) if err != nil { ctx.Logger().Error("timeout failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "timeout packet callback failed")) return nil, errorsmod.Wrap(err, "timeout packet callback failed") @@ -606,7 +606,7 @@ func (k *Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTime // If the timeout was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - err = k.ChannelKeeper.TimeoutOnClose(cacheCtx, capability, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv, msg.CounterpartyUpgradeSequence) + channelVersion, err := k.ChannelKeeper.TimeoutOnClose(cacheCtx, capability, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv, msg.CounterpartyUpgradeSequence) switch err { case nil: @@ -629,7 +629,7 @@ func (k *Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTime // // NOTE: MsgTimeout and MsgTimeoutOnClose use the same "OnTimeoutPacket" // application logic callback. - err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer) + err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer, channelVersion) if err != nil { ctx.Logger().Error("timeout on close failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "timeout on close callback failed")) return nil, errorsmod.Wrap(err, "timeout on close callback failed") @@ -671,7 +671,7 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck // If the acknowledgement was already received, perform a no-op // Use a cached context to prevent accidental state changes cacheCtx, writeFn := ctx.CacheContext() - err = k.ChannelKeeper.AcknowledgePacket(cacheCtx, capability, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight) + channelVersion, err := k.ChannelKeeper.AcknowledgePacket(cacheCtx, capability, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight) switch err { case nil: @@ -686,7 +686,7 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck } // Perform application logic callback - err = cbs.OnAcknowledgementPacket(ctx, msg.Packet, msg.Acknowledgement, relayer) + err = cbs.OnAcknowledgementPacket(ctx, msg.Packet, msg.Acknowledgement, relayer, channelVersion) if err != nil { ctx.Logger().Error("acknowledgement failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet callback failed")) return nil, errorsmod.Wrap(err, "acknowledge packet callback failed") diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index b99ebc84b94..3986ecdc497 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -71,6 +71,7 @@ type IBCApp struct { ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) exported.Acknowledgement OnAcknowledgementPacket func( @@ -78,12 +79,14 @@ type IBCApp struct { packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, + channelVersion string, ) error OnTimeoutPacket func( ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, + channelVersion string, ) error OnChanUpgradeInit func( diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index db7917d3c9b..f6645fa9bb1 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -123,9 +123,9 @@ func (im IBCModule) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string } // OnRecvPacket implements the IBCModule interface. -func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { +func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { - return im.IBCApp.OnRecvPacket(ctx, packet, relayer) + return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) } // set state by claiming capability to check if revert happens return @@ -148,9 +148,9 @@ func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re } // OnAcknowledgementPacket implements the IBCModule interface. -func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { +func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string) error { if im.IBCApp.OnAcknowledgementPacket != nil { - return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) } capName := GetMockAckCanaryCapabilityName(packet) @@ -166,9 +166,9 @@ func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes } // OnTimeoutPacket implements the IBCModule interface. -func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { +func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) error { if im.IBCApp.OnTimeoutPacket != nil { - return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer) + return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) } capName := GetMockTimeoutCanaryCapabilityName(packet) diff --git a/testing/mock/middleware.go b/testing/mock/middleware.go index 7a5b572cd0d..093f6bfb948 100644 --- a/testing/mock/middleware.go +++ b/testing/mock/middleware.go @@ -114,9 +114,9 @@ func (im BlockUpgradeMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, cha } // OnRecvPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { +func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { - return im.IBCApp.OnRecvPacket(ctx, packet, relayer) + return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) } // set state by claiming capability to check if revert happens return @@ -137,9 +137,9 @@ func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltyp } // OnAcknowledgementPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { +func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string) error { if im.IBCApp.OnAcknowledgementPacket != nil { - return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) } capName := GetMockAckCanaryCapabilityName(packet) @@ -153,9 +153,9 @@ func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet } // OnTimeoutPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) error { +func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) error { if im.IBCApp.OnTimeoutPacket != nil { - return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer) + return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) } capName := GetMockTimeoutCanaryCapabilityName(packet) From 72b5b979d8f4fe12e2bf096ffe947f898126d43d Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Mon, 22 Jul 2024 16:06:12 +0200 Subject: [PATCH 2/5] refactor 05-port module interface signature with channel version after ctx --- .../controller/ibc_middleware.go | 10 ++++---- .../controller/ibc_middleware_test.go | 6 ++--- .../27-interchain-accounts/host/ibc_module.go | 6 ++--- .../host/ibc_module_test.go | 6 ++--- modules/apps/29-fee/ibc_middleware.go | 24 +++++++++---------- modules/apps/29-fee/ibc_middleware_test.go | 6 ++--- modules/apps/callbacks/ibc_middleware.go | 12 +++++----- modules/apps/callbacks/ibc_middleware_test.go | 8 +++---- modules/apps/transfer/ibc_module.go | 6 ++--- modules/apps/transfer/ibc_module_test.go | 6 ++--- .../transfer/keeper/relay_forwarding_test.go | 2 +- modules/core/05-port/types/module.go | 6 ++--- modules/core/keeper/msg_server.go | 8 +++---- testing/mock/ibc_module.go | 6 ++--- testing/mock/middleware.go | 6 ++--- 15 files changed, 59 insertions(+), 59 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index b9192b28029..595930b231b 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -183,9 +183,9 @@ func (im IBCMiddleware) OnChanCloseConfirm( // OnRecvPacket implements the IBCMiddleware interface func (IBCMiddleware) OnRecvPacket( ctx sdk.Context, + _ string, packet channeltypes.Packet, _ sdk.AccAddress, - _ string, ) ibcexported.Acknowledgement { err := errorsmod.Wrapf(icatypes.ErrInvalidChannelFlow, "cannot receive packet on controller chain") ack := channeltypes.NewErrorAcknowledgement(err) @@ -196,10 +196,10 @@ 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, - channelVersion string, ) error { if !im.keeper.GetParams(ctx).ControllerEnabled { return types.ErrControllerSubModuleDisabled @@ -212,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, channelVersion) + return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } return nil @@ -221,9 +221,9 @@ func (im IBCMiddleware) OnAcknowledgementPacket( // OnTimeoutPacket implements the IBCMiddleware interface func (im IBCMiddleware) OnTimeoutPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) error { if !im.keeper.GetParams(ctx).ControllerEnabled { return types.ErrControllerSubModuleDisabled @@ -239,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, channelVersion) + return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } return nil diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 72dfc3b6eeb..21b7a142b6b 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -577,7 +577,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { ) ctx := suite.chainA.GetContext() - ack := cbs.OnRecvPacket(ctx, packet, nil, path.EndpointA.GetChannel().Version) + ack := cbs.OnRecvPacket(ctx, path.EndpointA.GetChannel().Version, packet, nil) suite.Require().Equal(tc.expPass, ack.Success()) expectedEvents := sdk.Events{ @@ -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, path.EndpointA.GetChannel().Version) + err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, []byte("ack"), nil) if tc.expPass { suite.Require().NoError(err) @@ -779,7 +779,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper) } - err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil, path.EndpointA.GetChannel().Version) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 622ed24048d..eba382a0561 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -113,9 +113,9 @@ func (im IBCModule) OnChanCloseConfirm( // OnRecvPacket implements the IBCModule interface func (im IBCModule) OnRecvPacket( ctx sdk.Context, + _ string, packet channeltypes.Packet, _ sdk.AccAddress, - _ string, ) ibcexported.Acknowledgement { if !im.keeper.GetParams(ctx).HostEnabled { im.keeper.Logger(ctx).Info("host submodule is disabled") @@ -142,10 +142,10 @@ 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, - channelVersion string, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot receive acknowledgement on a host channel end, a host chain does not send a packet over the channel") } @@ -153,9 +153,9 @@ func (IBCModule) OnAcknowledgementPacket( // OnTimeoutPacket implements the IBCModule interface func (IBCModule) OnTimeoutPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot cause a packet timeout on a host channel end, a host chain does not send a packet over the channel") } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 6bfc07a6890..12db8690f61 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -503,7 +503,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { suite.Require().True(ok) ctx := suite.chainB.GetContext() - ack := cbs.OnRecvPacket(ctx, packet, nil, path.EndpointB.GetChannel().Version) + ack := cbs.OnRecvPacket(ctx, path.EndpointB.GetChannel().Version, packet, nil) expectedAttributes := []sdk.Attribute{ sdk.NewAttribute(sdk.AttributeKeyModule, icatypes.ModuleName), @@ -587,7 +587,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() { 0, ) - err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), packet, []byte("ackBytes"), nil, path.EndpointB.GetChannel().Version) + err = cbs.OnAcknowledgementPacket(suite.chainB.GetContext(), path.EndpointB.GetChannel().Version, packet, []byte("ackBytes"), nil) if tc.expPass { suite.Require().NoError(err) @@ -642,7 +642,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { 0, ) - err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil, path.EndpointA.GetChannel().Version) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, nil) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 8e2976a6c66..bba7069ad4a 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -216,15 +216,15 @@ 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, - channelVersion string, ) exported.Acknowledgement { if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) { - return im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) + return im.app.OnRecvPacket(ctx, channelVersion, packet, relayer) } - ack := im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) + ack := im.app.OnRecvPacket(ctx, channelVersion, packet, relayer) // in case of async acknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement if ack == nil { @@ -242,13 +242,13 @@ 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, - channelVersion string, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) + return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } var ack types.IncentivizedAcknowledgement @@ -265,14 +265,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, channelVersion) + return im.app.OnAcknowledgementPacket(ctx, channelVersion, 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, channelVersion) + return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, ack.AppAcknowledgement, relayer) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -288,16 +288,16 @@ 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, channelVersion) + return im.app.OnAcknowledgementPacket(ctx, channelVersion, 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, - channelVersion string, ) error { // 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 @@ -305,14 +305,14 @@ func (im IBCMiddleware) OnTimeoutPacket( // // 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, channelVersion) + return im.app.OnTimeoutPacket(ctx, channelVersion, packet, 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.OnTimeoutPacket(ctx, packet, relayer, channelVersion) + return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -328,7 +328,7 @@ func (im IBCMiddleware) OnTimeoutPacket( im.keeper.DistributePacketFeesOnTimeout(ctx, payeeAddr, feesInEscrow.PacketFees, packetID) // call underlying callback - return im.app.OnTimeoutPacket(ctx, packet, relayer, channelVersion) + return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } // OnChanUpgradeInit implements the IBCModule interface diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index 17c4a249eb6..feb7e21027f 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -566,7 +566,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { // malleate test case tc.malleate() - result := cbs.OnRecvPacket(suite.chainB.GetContext(), packet, suite.chainA.SenderAccount.GetAddress(), suite.path.EndpointB.GetChannel().Version) + result := cbs.OnRecvPacket(suite.chainB.GetContext(), suite.path.EndpointB.GetChannel().Version, packet, suite.chainA.SenderAccount.GetAddress()) switch { case tc.name == "success": @@ -840,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, suite.path.EndpointA.GetChannel().Version) + err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, ack, relayerAddr) if tc.expPass { suite.Require().NoError(err) @@ -1051,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, suite.path.EndpointA.GetChannel().Version) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), suite.path.EndpointA.GetChannel().Version, packet, relayerAddr) if tc.expPass { suite.Require().NoError(err) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 7fe93920c9f..dfe21cc8547 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -130,13 +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, - channelVersion string, ) error { // we first call the underlying app to handle the acknowledgement - err := im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) + err := im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) if err != nil { return err } @@ -169,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, channelVersion string) error { - err := im.app.OnTimeoutPacket(ctx, packet, relayer, channelVersion) +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 } @@ -202,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, channelVersion string) ibcexported.Acknowledgement { - ack := im.app.OnRecvPacket(ctx, packet, relayer, channelVersion) +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. diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 880d10df072..e87c2317fd0 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -355,7 +355,7 @@ func (s *CallbacksTestSuite) TestOnAcknowledgementPacket() { s.Require().True(ok) onAcknowledgementPacket := func() error { - return transferStack.OnAcknowledgementPacket(ctx, packet, ack, s.chainA.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) + return transferStack.OnAcknowledgementPacket(ctx, s.path.EndpointA.GetChannel().Version, packet, ack, s.chainA.SenderAccount.GetAddress()) } switch tc.expError { @@ -519,7 +519,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() { s.Require().True(ok) onTimeoutPacket := func() error { - return transferStack.OnTimeoutPacket(ctx, packet, s.chainA.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) + return transferStack.OnTimeoutPacket(ctx, s.path.EndpointA.GetChannel().Version, packet, s.chainA.SenderAccount.GetAddress()) } switch expValue := tc.expValue.(type) { @@ -687,7 +687,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacket() { s.Require().True(ok) onRecvPacket := func() ibcexported.Acknowledgement { - return transferStack.OnRecvPacket(ctx, packet, s.chainB.SenderAccount.GetAddress(), s.path.EndpointA.GetChannel().Version) + return transferStack.OnRecvPacket(ctx, s.path.EndpointA.GetChannel().Version, packet, s.chainB.SenderAccount.GetAddress()) } switch tc.expAck { @@ -1132,7 +1132,7 @@ func (s *CallbacksTestSuite) TestOnRecvPacketAsyncAck() { 0, ) - ack := mockFeeCallbackStack.OnRecvPacket(s.chainA.GetContext(), packet, s.chainA.SenderAccount.GetAddress(), ibcmock.MockFeeVersion) + ack := mockFeeCallbackStack.OnRecvPacket(s.chainA.GetContext(), ibcmock.MockFeeVersion, packet, s.chainA.SenderAccount.GetAddress()) s.Require().Nil(ack) s.AssertHasExecutedExpectedCallback("none", true) } diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index a0607051917..64606e3ebc6 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -180,9 +180,9 @@ func (IBCModule) OnChanCloseConfirm( // A nil acknowledgement may be returned when using the packet forwarding feature. This signals to core IBC that the acknowledgement will be written asynchronously. func (im IBCModule) OnRecvPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) ibcexported.Acknowledgement { var ( ackErr error @@ -224,10 +224,10 @@ func (im IBCModule) OnRecvPacket( // OnAcknowledgementPacket implements the IBCModule interface func (im IBCModule) OnAcknowledgementPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, - channelVersion string, ) error { var ack channeltypes.Acknowledgement if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { @@ -251,9 +251,9 @@ func (im IBCModule) OnAcknowledgementPacket( // OnTimeoutPacket implements the IBCModule interface func (im IBCModule) OnTimeoutPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) error { data, err := im.getICS20PacketData(ctx, packet.GetData(), packet.GetSourcePort(), packet.GetSourceChannel()) if err != nil { diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index f3d59f93fb6..355b8737d63 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -403,7 +403,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() { tc.malleate() // change fields in packet - ack := cbs.OnRecvPacket(ctx, packet, suite.chainB.SenderAccount.GetAddress(), path.EndpointB.GetChannel().Version) + ack := cbs.OnRecvPacket(ctx, path.EndpointB.GetChannel().Version, packet, suite.chainB.SenderAccount.GetAddress()) suite.Require().Equal(tc.expAck, ack) @@ -468,7 +468,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() { cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module) suite.Require().True(ok) - suite.Require().NoError(cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress(), path.EndpointA.GetChannel().Version)) + suite.Require().NoError(cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, suite.chainA.SenderAccount.GetAddress())) }, errors.New("unable to unescrow tokens"), }, @@ -508,7 +508,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() { tc.malleate() // change fields in packet - err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, suite.chainA.SenderAccount.GetAddress(), path.EndpointA.GetChannel().Version) + err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), path.EndpointA.GetChannel().Version, packet, suite.chainA.SenderAccount.GetAddress()) expPass := tc.expError == nil if expPass { diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index 9336b13a1cb..542af7a4f59 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -982,7 +982,7 @@ func (suite *ForwardingTestSuite) TestOnTimeoutPacketForwarding() { suite.Require().True(ok) // Trigger OnTimeoutPacket for chainB - err = cbs.OnTimeoutPacket(suite.chainB.GetContext(), packet, nil, pathBtoC.EndpointA.GetChannel().Version) + err = cbs.OnTimeoutPacket(suite.chainB.GetContext(), pathBtoC.EndpointA.GetChannel().Version, packet, nil) suite.Require().NoError(err) // Ensure that chainB has an ack. diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index bfa77480dba..8b2ccf94248 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -88,24 +88,24 @@ type IBCModule interface { // and the acknowledgement is written (in synchronous cases). OnRecvPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) exported.Acknowledgement OnAcknowledgementPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, - channelVersion string, ) error OnTimeoutPacket( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) error } diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 6e2e68826a3..2f9a2acc7be 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -491,7 +491,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack // // Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful. cacheCtx, writeFn = ctx.CacheContext() - ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer, channelVersion) + ack := cbs.OnRecvPacket(cacheCtx, channelVersion, msg.Packet, relayer) if ack == nil || ack.Success() { // write application state changes for asynchronous and successful acknowledgements writeFn() @@ -565,7 +565,7 @@ func (k *Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (* } // Perform application logic callback - err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer, channelVersion) + err = cbs.OnTimeoutPacket(ctx, channelVersion, msg.Packet, relayer) if err != nil { ctx.Logger().Error("timeout failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "timeout packet callback failed")) return nil, errorsmod.Wrap(err, "timeout packet callback failed") @@ -630,7 +630,7 @@ func (k *Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTime // // NOTE: MsgTimeout and MsgTimeoutOnClose use the same "OnTimeoutPacket" // application logic callback. - err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer, channelVersion) + err = cbs.OnTimeoutPacket(ctx, channelVersion, msg.Packet, relayer) if err != nil { ctx.Logger().Error("timeout on close failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "timeout on close callback failed")) return nil, errorsmod.Wrap(err, "timeout on close callback failed") @@ -687,7 +687,7 @@ func (k *Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAck } // Perform application logic callback - err = cbs.OnAcknowledgementPacket(ctx, msg.Packet, msg.Acknowledgement, relayer, channelVersion) + err = cbs.OnAcknowledgementPacket(ctx, channelVersion, msg.Packet, msg.Acknowledgement, relayer) if err != nil { ctx.Logger().Error("acknowledgement failed", "port-id", msg.Packet.SourcePort, "channel-id", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet callback failed")) return nil, errorsmod.Wrap(err, "acknowledge packet callback failed") diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index f6645fa9bb1..3e858edc777 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -123,7 +123,7 @@ func (im IBCModule) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string } // OnRecvPacket implements the IBCModule interface. -func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) exported.Acknowledgement { +func (im IBCModule) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) } @@ -148,7 +148,7 @@ func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re } // OnAcknowledgementPacket implements the IBCModule interface. -func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string) error { +func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { if im.IBCApp.OnAcknowledgementPacket != nil { return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) } @@ -166,7 +166,7 @@ func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes } // OnTimeoutPacket implements the IBCModule interface. -func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) error { +func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { if im.IBCApp.OnTimeoutPacket != nil { return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) } diff --git a/testing/mock/middleware.go b/testing/mock/middleware.go index 093f6bfb948..b41103c4861 100644 --- a/testing/mock/middleware.go +++ b/testing/mock/middleware.go @@ -114,7 +114,7 @@ func (im BlockUpgradeMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, cha } // OnRecvPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) exported.Acknowledgement { +func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) } @@ -137,7 +137,7 @@ func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, packet channeltyp } // OnAcknowledgementPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, channelVersion string) error { +func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { if im.IBCApp.OnAcknowledgementPacket != nil { return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) } @@ -153,7 +153,7 @@ func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, packet } // OnTimeoutPacket implements the IBCModule interface. -func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string) error { +func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { if im.IBCApp.OnTimeoutPacket != nil { return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) } From c41c821dbcc208cd15e5f68182432bd44f7fe089 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Tue, 23 Jul 2024 14:03:22 +0200 Subject: [PATCH 3/5] unwrap app version in ics29 --- modules/apps/29-fee/ibc_middleware.go | 43 +++++++++++++++++++----- modules/apps/transfer/ibc_module.go | 14 +++----- modules/apps/transfer/ibc_module_test.go | 2 +- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index bba7069ad4a..39dcb57d80c 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -224,7 +224,11 @@ func (im IBCMiddleware) OnRecvPacket( return im.app.OnRecvPacket(ctx, channelVersion, packet, relayer) } - ack := im.app.OnRecvPacket(ctx, channelVersion, packet, relayer) + appVersion, err := unwrapAppVersion(channelVersion) + if err != nil { + return channeltypes.NewErrorAcknowledgement(err) + } + 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 { @@ -251,6 +255,11 @@ func (im IBCMiddleware) OnAcknowledgementPacket( return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } + appVersion, err := unwrapAppVersion(channelVersion) + if err != nil { + return err + } + var ack types.IncentivizedAcknowledgement if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { return errorsmod.Wrapf(err, "cannot unmarshal ICS-29 incentivized packet acknowledgement: %v", ack) @@ -265,14 +274,14 @@ func (im IBCMiddleware) OnAcknowledgementPacket( // for fee enabled channels // // Please see ADR 004 for more information. - return im.app.OnAcknowledgementPacket(ctx, channelVersion, 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, channelVersion, packet, ack.AppAcknowledgement, relayer) + return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -288,7 +297,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( im.keeper.DistributePacketFeesOnAcknowledgement(ctx, ack.ForwardRelayerAddress, payeeAddr, feesInEscrow.PacketFees, packetID) // call underlying callback - return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, ack.AppAcknowledgement, relayer) + return im.app.OnAcknowledgementPacket(ctx, appVersion, packet, ack.AppAcknowledgement, relayer) } // OnTimeoutPacket implements the IBCMiddleware interface @@ -299,20 +308,29 @@ func (im IBCMiddleware) OnTimeoutPacket( 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, channelVersion, packet, relayer) + if im.keeper.IsLocked(ctx) { + return im.app.OnTimeoutPacket(ctx, appVersion, packet, 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.OnTimeoutPacket(ctx, channelVersion, packet, relayer) + return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer) } payee, found := im.keeper.GetPayeeAddress(ctx, relayer.String(), packet.SourceChannel) @@ -328,7 +346,7 @@ func (im IBCMiddleware) OnTimeoutPacket( im.keeper.DistributePacketFeesOnTimeout(ctx, payeeAddr, feesInEscrow.PacketFees, packetID) // call underlying callback - return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer) + return im.app.OnTimeoutPacket(ctx, appVersion, packet, relayer) } // OnChanUpgradeInit implements the IBCModule interface @@ -483,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 +} diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 64606e3ebc6..4af345c339a 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -197,7 +197,7 @@ func (im IBCModule) OnRecvPacket( events.EmitOnRecvPacketEvent(ctx, data, ack, ackErr) }() - data, ackErr = im.getICS20PacketData(ctx, packet.GetData(), packet.GetDestPort(), packet.GetDestChannel()) + data, ackErr = types.UnmarshalPacketData(packet.GetData(), channelVersion) if ackErr != nil { ack = channeltypes.NewErrorAcknowledgement(ackErr) im.keeper.Logger(ctx).Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence)) @@ -234,7 +234,7 @@ func (im IBCModule) OnAcknowledgementPacket( return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) } - data, err := im.getICS20PacketData(ctx, packet.GetData(), packet.GetSourcePort(), packet.GetSourceChannel()) + data, err := types.UnmarshalPacketData(packet.GetData(), channelVersion) if err != nil { return err } @@ -255,7 +255,7 @@ func (im IBCModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { - data, err := im.getICS20PacketData(ctx, packet.GetData(), packet.GetSourcePort(), packet.GetSourceChannel()) + data, err := types.UnmarshalPacketData(packet.GetData(), channelVersion) if err != nil { return err } @@ -313,16 +313,10 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr // into a FungibleTokenPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. func (im IBCModule) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) { - return im.getICS20PacketData(ctx, bz, portID, channelID) -} - -// getICS20PacketData determines the current version of ICS20 and unmarshals the packet data into either a FungibleTokenPacketData -// or a FungibleTokenPacketDataV2 depending on the application version. -func (im IBCModule) getICS20PacketData(ctx sdk.Context, packetData []byte, portID, channelID string) (types.FungibleTokenPacketDataV2, error) { ics20Version, found := im.keeper.GetICS4Wrapper().GetAppVersion(ctx, portID, channelID) if !found { return types.FungibleTokenPacketDataV2{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) } - return types.UnmarshalPacketData(packetData, ics20Version) + return types.UnmarshalPacketData(bz, ics20Version) } diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 355b8737d63..4dd4a505fd5 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -448,7 +448,7 @@ func (suite *TransferTestSuite) TestOnTimeoutPacket() { func() { packet.SourceChannel = "channel-100" }, - ibcerrors.ErrNotFound, + errors.New("unable to unescrow tokens"), }, { "invalid packet data", From 24793c13fc2721a92a7f27619fb085f3a14b4638 Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Wed, 24 Jul 2024 12:31:14 +0200 Subject: [PATCH 4/5] panic on unwrap app version and update mock apis --- .../controller/ibc_middleware_test.go | 8 +++---- .../host/ibc_module_test.go | 2 +- modules/apps/29-fee/ibc_middleware.go | 22 ++++++------------- modules/apps/29-fee/ibc_middleware_test.go | 6 ++--- modules/core/ante/ante_test.go | 4 ++-- testing/mock/ibc_app.go | 6 ++--- testing/mock/ibc_module.go | 6 ++--- testing/mock/middleware.go | 6 ++--- 8 files changed, 26 insertions(+), 34 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 21b7a142b6b..420771e8f5c 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -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, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, ) error { return fmt.Errorf("mock ica auth fails") } @@ -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, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, ) error { return fmt.Errorf("error should be unreachable") } @@ -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, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, ) error { return fmt.Errorf("mock ica auth fails") } @@ -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, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, ) error { return fmt.Errorf("error should be unreachable") } diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 12db8690f61..cce0a8c76ee 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -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, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, ) exported.Acknowledgement { return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed OnRecvPacket mock callback")) } diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 39dcb57d80c..97ec4fad60a 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -224,10 +224,7 @@ func (im IBCMiddleware) OnRecvPacket( return im.app.OnRecvPacket(ctx, channelVersion, packet, relayer) } - appVersion, err := unwrapAppVersion(channelVersion) - if err != nil { - return channeltypes.NewErrorAcknowledgement(err) - } + appVersion := unwrapAppVersion(channelVersion) 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 @@ -255,10 +252,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( return im.app.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } - appVersion, err := unwrapAppVersion(channelVersion) - if err != nil { - return err - } + appVersion := unwrapAppVersion(channelVersion) var ack types.IncentivizedAcknowledgement if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { @@ -312,10 +306,7 @@ func (im IBCMiddleware) OnTimeoutPacket( return im.app.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } - appVersion, err := unwrapAppVersion(channelVersion) - if err != nil { - return err - } + appVersion := unwrapAppVersion(channelVersion) // 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 @@ -502,11 +493,12 @@ func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID, channelID s return unmarshaler.UnmarshalPacketData(ctx, portID, channelID, bz) } -func unwrapAppVersion(channelVersion string) (string, error) { +func unwrapAppVersion(channelVersion string) string { metadata, err := types.MetadataFromVersion(channelVersion) if err != nil { - return "", err + // This should not happen, as it would mean that the channel is broken. Only a severe bug would cause this. + panic(errorsmod.Wrap(err, "failed to unwrap app version from channel version")) } - return metadata.AppVersion, nil + return metadata.AppVersion } diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index feb7e21027f..8f24197a2c7 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -513,9 +513,9 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { // setup mock callback suite.chainB.GetSimApp().FeeMockModule.IBCApp.OnRecvPacket = func( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) exported.Acknowledgement { return nil } @@ -799,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, _ string) error { + suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnAcknowledgementPacket = func(_ sdk.Context, _ string, _ channeltypes.Packet, _ []byte, _ sdk.AccAddress) error { return fmt.Errorf("mock fee app callback fails") } }, @@ -1013,7 +1013,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() { { "application callback fails", func() { - suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnTimeoutPacket = func(_ sdk.Context, _ channeltypes.Packet, _ sdk.AccAddress, _ string) error { + suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnTimeoutPacket = func(_ sdk.Context, _ string, _ channeltypes.Packet, _ sdk.AccAddress) error { return fmt.Errorf("mock fee app callback fails") } }, diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go index f852896475b..1757d3d3534 100644 --- a/modules/core/ante/ante_test.go +++ b/modules/core/ante/ante_test.go @@ -351,7 +351,7 @@ func (suite *AnteTestSuite) TestAnteDecoratorCheckTx() { "success on app callback error, app callbacks are skipped for performance", func(suite *AnteTestSuite) []sdk.Msg { suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func( - ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, ) exported.Acknowledgement { panic(fmt.Errorf("failed OnRecvPacket mock callback")) } @@ -587,7 +587,7 @@ func (suite *AnteTestSuite) TestAnteDecoratorReCheckTx() { "success on app callback error, app callbacks are skipped for performance", func(suite *AnteTestSuite) []sdk.Msg { suite.chainB.GetSimApp().IBCMockModule.IBCApp.OnRecvPacket = func( - ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress, channelVersion string, + ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, ) exported.Acknowledgement { panic(fmt.Errorf("failed OnRecvPacket mock callback")) } diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index 3986ecdc497..798bd35749a 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -69,24 +69,24 @@ type IBCApp struct { // and the acknowledgement is written (in synchronous cases). OnRecvPacket func( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) exported.Acknowledgement OnAcknowledgementPacket func( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress, - channelVersion string, ) error OnTimeoutPacket func( ctx sdk.Context, + channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress, - channelVersion string, ) error OnChanUpgradeInit func( diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index 3e858edc777..c698c05d1bd 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -125,7 +125,7 @@ func (im IBCModule) OnChanCloseConfirm(ctx sdk.Context, portID, channelID string // OnRecvPacket implements the IBCModule interface. func (im IBCModule) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { - return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) + return im.IBCApp.OnRecvPacket(ctx, channelVersion, packet, relayer) } // set state by claiming capability to check if revert happens return @@ -150,7 +150,7 @@ func (im IBCModule) OnRecvPacket(ctx sdk.Context, channelVersion string, packet // OnAcknowledgementPacket implements the IBCModule interface. func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { if im.IBCApp.OnAcknowledgementPacket != nil { - return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) + return im.IBCApp.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } capName := GetMockAckCanaryCapabilityName(packet) @@ -168,7 +168,7 @@ func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, channelVersion stri // OnTimeoutPacket implements the IBCModule interface. func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { if im.IBCApp.OnTimeoutPacket != nil { - return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) + return im.IBCApp.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } capName := GetMockTimeoutCanaryCapabilityName(packet) diff --git a/testing/mock/middleware.go b/testing/mock/middleware.go index b41103c4861..6d6a33a1a1f 100644 --- a/testing/mock/middleware.go +++ b/testing/mock/middleware.go @@ -116,7 +116,7 @@ func (im BlockUpgradeMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, cha // OnRecvPacket implements the IBCModule interface. func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement { if im.IBCApp.OnRecvPacket != nil { - return im.IBCApp.OnRecvPacket(ctx, packet, relayer, channelVersion) + return im.IBCApp.OnRecvPacket(ctx, channelVersion, packet, relayer) } // set state by claiming capability to check if revert happens return @@ -139,7 +139,7 @@ func (im BlockUpgradeMiddleware) OnRecvPacket(ctx sdk.Context, channelVersion st // OnAcknowledgementPacket implements the IBCModule interface. func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, acknowledgement []byte, relayer sdk.AccAddress) error { if im.IBCApp.OnAcknowledgementPacket != nil { - return im.IBCApp.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer, channelVersion) + return im.IBCApp.OnAcknowledgementPacket(ctx, channelVersion, packet, acknowledgement, relayer) } capName := GetMockAckCanaryCapabilityName(packet) @@ -155,7 +155,7 @@ func (im BlockUpgradeMiddleware) OnAcknowledgementPacket(ctx sdk.Context, channe // OnTimeoutPacket implements the IBCModule interface. func (im BlockUpgradeMiddleware) OnTimeoutPacket(ctx sdk.Context, channelVersion string, packet channeltypes.Packet, relayer sdk.AccAddress) error { if im.IBCApp.OnTimeoutPacket != nil { - return im.IBCApp.OnTimeoutPacket(ctx, packet, relayer, channelVersion) + return im.IBCApp.OnTimeoutPacket(ctx, channelVersion, packet, relayer) } capName := GetMockTimeoutCanaryCapabilityName(packet) From d4ff68933b3f4075dd2cfd48f3815791c2d20efc Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Wed, 24 Jul 2024 12:42:37 +0200 Subject: [PATCH 5/5] discard unused variables --- .../27-interchain-accounts/host/ibc_module.go | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index eba382a0561..18339a7f9f8 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -37,14 +37,14 @@ func NewIBCModule(k keeper.Keeper) IBCModule { // OnChanOpenInit implements the IBCModule interface func (IBCModule) OnChanOpenInit( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID string, - channelID string, - chanCap *capabilitytypes.Capability, - counterparty channeltypes.Counterparty, - version string, + _ sdk.Context, + _ channeltypes.Order, + _ []string, + _ string, + _ string, + _ *capabilitytypes.Capability, + _ channeltypes.Counterparty, + _ string, ) (string, error) { return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain") } @@ -69,11 +69,11 @@ func (im IBCModule) OnChanOpenTry( // OnChanOpenAck implements the IBCModule interface func (IBCModule) OnChanOpenAck( - ctx sdk.Context, - portID, - channelID string, - counterpartyChannelID string, - counterpartyVersion string, + _ sdk.Context, + _, + _ string, + _ string, + _ string, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain") } @@ -93,9 +93,9 @@ func (im IBCModule) OnChanOpenConfirm( // OnChanCloseInit implements the IBCModule interface func (IBCModule) OnChanCloseInit( - ctx sdk.Context, - portID, - channelID string, + _ sdk.Context, + _ string, + _ string, ) error { // Disallow user-initiated channel closing for interchain account channels return errorsmod.Wrap(ibcerrors.ErrInvalidRequest, "user cannot close channel") @@ -141,27 +141,27 @@ 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, + _ sdk.Context, + _ string, + _ channeltypes.Packet, + _ []byte, + _ sdk.AccAddress, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot receive acknowledgement on a host channel end, a host chain does not send a packet over the channel") } // OnTimeoutPacket implements the IBCModule interface func (IBCModule) OnTimeoutPacket( - ctx sdk.Context, - channelVersion string, - packet channeltypes.Packet, - relayer sdk.AccAddress, + _ sdk.Context, + _ string, + _ channeltypes.Packet, + _ sdk.AccAddress, ) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "cannot cause a packet timeout on a host channel end, a host chain does not send a packet over the channel") } // OnChanUpgradeInit implements the IBCModule interface -func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) { +func (IBCModule) OnChanUpgradeInit(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) (string, error) { return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel upgrade handshake must be initiated by controller chain") } @@ -175,12 +175,12 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, } // OnChanUpgradeAck implements the IBCModule interface -func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error { +func (IBCModule) OnChanUpgradeAck(_ sdk.Context, _, _, _ string) error { return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel upgrade handshake must be initiated by controller chain") } // OnChanUpgradeOpen implements the IBCModule interface -func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) { +func (IBCModule) OnChanUpgradeOpen(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) { } // UnmarshalPacketData attempts to unmarshal the provided packet data bytes