Merge branch 'main' into cian/issue#2823-fee-events

docs/docs/01-ibc/06-channel-upgrades.md

@@ -70,6 +70,10 @@ As part of the handling of the `MsgChannelUpgradeInit` message, the application'
 
 After this message is handled successfully, the channel's upgrade sequence will be incremented. This upgrade sequence will serve as a nonce for the upgrade process to provide replay protection.
 
+::: warning
+Initiating an upgrade in the same block as opening a channel may potentially prevent the counterparty channel from also opening. 
+:::
+
 ### Governance gating on `ChanUpgradeInit`
 
 The message signer for `MsgChannelUpgradeInit` must be the address which has been designated as the `authority` of the `IBCKeeper`. If this proposal passes, the counterparty's channel will upgrade by default.
@@ -132,6 +136,15 @@ The state will change to `FLUSHCOMPLETE` once there are no in-flight packets lef
 
 All other parameters will remain the same during the upgrade handshake until the upgrade handshake completes. When the channel is reset to `OPEN` on a successful upgrade handshake, the relevant fields on the channel end will be switched over to the `UpgradeFields` specified in the upgrade.
 
+:::warning
+
+Due to the addition of new channel states, packets can still be received and processed in both `FLUSHING` and `FLUSHCOMPLETE` states.
+Packets can also be acknowledged in the `FLUSHING` state. Acknowledging will **not** be possible when the channel is in the `FLUSHCOMPLETE` state, since all packets sent from that channel end have been flushed.
+Application developers should consider these new states when implementing application logic that relies on the channel state.
+It is still only possible to send packets when the channel is in the `OPEN` state, but sending is disallowed when the channel enters `FLUSHING` and `FLUSHCOMPLETE`. When the channel reopens, sending will be possible again.
+
+:::
+
 ## Cancelling a Channel Upgrade
 
 Channel upgrade cancellation is performed by submitting a `MsgChannelUpgradeCancel` message.
@@ -152,8 +165,6 @@ It is possible for a relayer to cancel an in-progress channel upgrade if the fol
 Upon cancelling a channel upgrade, an `ErrorReceipt` will be written with the channel's current upgrade sequence, and
 the channel will move back to `OPEN` state keeping its original parameters.
 
-The application's `OnChanUpgradeRestore` callback method will be invoked.
-
 It will then be possible to re-initiate an upgrade by sending a `MsgChannelOpenInit` message.
 
 ## Timing Out a Channel Upgrade
@@ -204,8 +215,6 @@ type MsgChannelUpgradeTimeout struct {
 
 An `ErrorReceipt` will be written with the channel's current upgrade sequence, and the channel will move back to `OPEN` state keeping its original parameters.
 
-The application's `OnChanUpgradeRestore` callback method will also be invoked.
-
 Note that timing out a channel upgrade will end the upgrade process, and a new `MsgChannelUpgradeInit` will have to be submitted via governance in order to restart the upgrade process.
 
 ## Pruning Acknowledgements
@@ -241,7 +250,8 @@ simd tx ibc channel prune-acknowledgements [port] [channel] [limit]
 
 ## IBC App Recommendations
 
-IBC application callbacks should be primarily used to validate data fields and do compatibility checks.
+IBC application callbacks should be primarily used to validate data fields and do compatibility checks. Application developers
+should be aware that callbacks will be invoked before any core ibc state changes are written.
 
 `OnChanUpgradeInit` should validate the proposed version, order, and connection hops, and should return the application version to upgrade to.
 
@@ -251,8 +261,6 @@ IBC application callbacks should be primarily used to validate data fields and d
 
 `OnChanUpgradeOpen` should perform any logic associated with changing of the channel fields.
 
-`OnChanUpgradeRestore`  should perform any logic that needs to be executed when an upgrade attempt fails as is reverted.
-
 > IBC applications should not attempt to process any packet data under the new conditions until after `OnChanUpgradeOpen`
 > has been executed, as up until this point it is still possible for the upgrade handshake to fail and for the channel
 > to remain in the pre-upgraded state. 

docs/docs/05-migrations/13-v8-to-v8_1.md → docs/docs/05-migrations/12-v8-to-v8_1.md

@@ -1,7 +1,7 @@
 ---
 title: IBC-Go v8 to v8.1
 sidebar_label: IBC-Go v8 to v8.1
-sidebar_position: 9
+sidebar_position: 12
 slug: /migrations/v8-to-v8_1
 ---
 

docs/docs/05-migrations/12-v8-to-v9.md → docs/docs/05-migrations/13-v8-to-v9.md

@@ -1,7 +1,7 @@
 ---
 title: IBC-Go v8 to v9
 sidebar_label: IBC-Go v8 to v9
-sidebar_position: 12
+sidebar_position: 13
 slug: /migrations/v8-to-v9
 ---
 

modules/apps/27-interchain-accounts/controller/ibc_middleware.go

@@ -309,23 +309,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str
 	}
 }
 
-// OnChanUpgradeRestore implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {
-	connectionID, err := im.keeper.GetConnectionID(ctx, portID, channelID)
-	if err != nil {
-		panic(err)
-	}
-
-	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
-		// Only cast to UpgradableModule if the application is set.
-		cbs, ok := im.app.(porttypes.UpgradableModule)
-		if !ok {
-			panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack"))
-		}
-		cbs.OnChanUpgradeRestore(ctx, portID, channelID)
-	}
-}
-
 // SendPacket implements the ICS4 Wrapper interface
 func (IBCMiddleware) SendPacket(
 	ctx sdk.Context,

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -1080,67 +1080,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() {
 	}
 }
 
-func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeRestore() {
-	var (
-		path     *ibctesting.Path
-		isNilApp bool
-	)
-
-	testCases := []struct {
-		name     string
-		malleate func()
-	}{
-		{
-			"success", func() {},
-		},
-		{
-			"success: nil app",
-			func() {
-				isNilApp = true
-			},
-		},
-		{
-			"middleware disabled", func() {
-				suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)
-				suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanUpgradeAck = func(ctx sdk.Context, portID, channelID string, counterpartyVersion string) error {
-					return ibcmock.MockApplicationCallbackError
-				}
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-
-		suite.Run(tc.name, func() {
-			suite.SetupTest() // reset
-			isNilApp = false
-
-			path = NewICAPath(suite.chainA, suite.chainB)
-			suite.coordinator.SetupConnections(path)
-
-			err := SetupICAPath(path, TestOwnerAddress)
-			suite.Require().NoError(err)
-
-			tc.malleate() // malleate mutates test data
-
-			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
-			suite.Require().NoError(err)
-
-			app, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
-			suite.Require().True(ok)
-			cbs, ok := app.(porttypes.UpgradableModule)
-			suite.Require().True(ok)
-
-			if isNilApp {
-				cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
-			}
-
-			cbs.OnChanUpgradeRestore(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
-		})
-	}
-}
-
 func (suite *InterchainAccountsTestSuite) TestSingleHostMultipleControllers() {
 	var (
 		pathAToB *ibctesting.Path

modules/apps/27-interchain-accounts/controller/keeper/keeper.go

@@ -150,7 +150,7 @@ func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID strin
 
 	channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
 
-	if found && channel.IsOpen() {
+	if found && channel.State == channeltypes.OPEN {
 		return channelID, true
 	}
 
@@ -165,7 +165,7 @@ func (k Keeper) IsActiveChannelClosed(ctx sdk.Context, connectionID, portID stri
 	}
 
 	channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
-	return found && channel.IsClosed()
+	return found && channel.State == channeltypes.CLOSED
 }
 
 // GetAllActiveChannels returns a list of all active interchain accounts controller channels and their associated connection and port identifiers

modules/apps/27-interchain-accounts/host/ibc_module.go

@@ -180,9 +180,6 @@ func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpar
 func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) {
 }
 
-// OnChanUpgradeRestore implements the IBCModule interface
-func (IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {}
-
 // UnmarshalPacketData attempts to unmarshal the provided packet data bytes
 // into an InterchainAccountPacketData. This function implements the optional
 // PacketDataUnmarshaler interface required for ADR 008 support.

modules/apps/27-interchain-accounts/host/keeper/keeper.go

@@ -152,7 +152,7 @@ func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID strin
 
 	channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
 
-	if found && channel.IsOpen() {
+	if found && channel.State == channeltypes.OPEN {
 		return channelID, true
 	}
 

modules/apps/29-fee/ibc_middleware.go

@@ -440,16 +440,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str
 	cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, versionMetadata.AppVersion)
 }
 
-// OnChanUpgradeRestore implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack"))
-	}
-
-	cbs.OnChanUpgradeRestore(ctx, portID, channelID)
-}
-
 // SendPacket implements the ICS4 Wrapper interface
 func (im IBCMiddleware) SendPacket(
 	ctx sdk.Context,

modules/apps/29-fee/keeper/migrations_test.go

@@ -20,14 +20,18 @@ func (suite *KeeperTestSuite) TestLegacyTotal() {
 
 func (suite *KeeperTestSuite) TestMigrate1to2() {
 	var (
+		packetFee        types.PacketFee
+		packetFees       []types.PacketFee
 		packetID         channeltypes.PacketId
+		packetID2        channeltypes.PacketId
 		moduleAcc        sdk.AccAddress
 		refundAcc        sdk.AccAddress
 		initRefundAccBal sdk.Coins
 		initModuleAccBal sdk.Coins
-		packetFees       []types.PacketFee
 	)
 
+	fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
+
 	testCases := []struct {
 		name     string
 		malleate func()
@@ -52,8 +56,6 @@ func (suite *KeeperTestSuite) TestMigrate1to2() {
 		{
 			"success: one fee in escrow",
 			func() {
-				fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
-				packetFee := types.NewPacketFee(fee, refundAcc.String(), []string(nil))
 				packetFees = []types.PacketFee{packetFee}
 			},
 			func(err error) {
@@ -65,9 +67,10 @@ func (suite *KeeperTestSuite) TestMigrate1to2() {
 				}
 				suite.Require().Equal(expPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetAllIdentifiedPacketFees(suite.chainA.GetContext()))
 
-				unusedFee := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(300))
+				unusedFee := keeper.LegacyTotal(fee).Sub(packetFee.Fee.Total()...)[0]
 				// refund account balance should increase
 				refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom)
+
 				suite.Require().Equal(initRefundAccBal.Add(unusedFee)[0], refundAccBal)
 
 				// module account balance should decrease
@@ -78,21 +81,19 @@ func (suite *KeeperTestSuite) TestMigrate1to2() {
 		{
 			"success: many fees with multiple denoms in escrow",
 			func() {
-				fee1 := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
-				packetFee1 := types.NewPacketFee(fee1, refundAcc.String(), []string(nil))
-
-				// mint some tokens to the refund account
+				// mint second denom tokens to the refund account
 				denom2 := "denom"
 				err := suite.chainA.GetSimApp().MintKeeper.MintCoins(suite.chainA.GetContext(), sdk.NewCoins(sdk.NewCoin(denom2, sdkmath.NewInt(1000))))
 				suite.Require().NoError(err)
 				err = suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), minttypes.ModuleName, refundAcc, sdk.NewCoins(sdk.NewCoin(denom2, sdkmath.NewInt(1000))))
 				suite.Require().NoError(err)
 
+				// assemble second denom type packet
 				defaultFee2 := sdk.NewCoins(sdk.NewCoin(denom2, sdkmath.NewInt(100)))
 				fee2 := types.NewFee(defaultFee2, defaultFee2, defaultFee2)
 				packetFee2 := types.NewPacketFee(fee2, refundAcc.String(), []string(nil))
 
-				packetFees = []types.PacketFee{packetFee1, packetFee2, packetFee1}
+				packetFees = []types.PacketFee{packetFee, packetFee2, packetFee}
 			},
 			func(err error) {
 				denom2 := "denom"
@@ -118,11 +119,43 @@ func (suite *KeeperTestSuite) TestMigrate1to2() {
 				suite.Require().Equal(initModuleAccBal.Sub(unusedFee...).Sort(), moduleAccBal)
 			},
 		},
+		{
+			"success: more than one packet",
+			func() {
+				packetFees = []types.PacketFee{packetFee}
+
+				// add second packet to have escrowed fees
+				packetID2 = channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 2)
+				// escrow the packet fee for the second packet & store the fee in state
+				suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID2, types.NewPacketFees(packetFees))
+				err := suite.chainA.GetSimApp().BankKeeper.SendCoinsFromAccountToModule(suite.chainA.GetContext(), refundAcc, types.ModuleName, keeper.LegacyTotal(packetFee.Fee))
+				suite.Require().NoError(err)
+			},
+			func(err error) {
+				suite.Require().NoError(err)
+
+				// ensure that the packet fees are unmodified
+				expPacketFees := []types.IdentifiedPacketFees{
+					types.NewIdentifiedPacketFees(packetID, packetFees),
+					types.NewIdentifiedPacketFees(packetID2, packetFees),
+				}
+				suite.Require().Equal(expPacketFees, suite.chainA.GetSimApp().IBCFeeKeeper.GetAllIdentifiedPacketFees(suite.chainA.GetContext()))
+
+				// 300 for each packet
+				unusedFee := keeper.LegacyTotal(fee).Sub(packetFee.Fee.Total()...).MulInt(sdkmath.NewInt(2))[0]
+				// refund account balance should increase
+				refundAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), refundAcc, sdk.DefaultBondDenom)
+				suite.Require().Equal(initRefundAccBal.Add(unusedFee)[0], refundAccBal)
+
+				// module account balance should decrease
+				moduleAccBal := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), moduleAcc, sdk.DefaultBondDenom)
+				suite.Require().Equal(initModuleAccBal.Sub(unusedFee)[0], moduleAccBal)
+			},
+		},
 		{
 			"failure: invalid refund address",
 			func() {
-				fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
-				packetFee := types.NewPacketFee(fee, "invalid", []string{})
+				packetFee = types.NewPacketFee(fee, "invalid", []string{})
 				packetFees = []types.PacketFee{packetFee}
 			},
 			func(err error) {
@@ -138,6 +171,7 @@ func (suite *KeeperTestSuite) TestMigrate1to2() {
 		suite.coordinator.Setup(suite.path)
 
 		refundAcc = suite.chainA.SenderAccount.GetAddress()
+		packetFee = types.NewPacketFee(fee, refundAcc.String(), []string(nil))
 		moduleAcc = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName)
 		packetID = channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1)
 		packetFees = nil

modules/apps/callbacks/ibc_middleware.go

@@ -404,16 +404,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str
 	cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
 }
 
-// OnChanUpgradeRestore implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack"))
-	}
-
-	cbs.OnChanUpgradeRestore(ctx, portID, channelID)
-}
-
 // GetAppVersion implements the ICS4Wrapper interface. Callbacks has no version,
 // so the call is deferred to the underlying application.
 func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {

modules/apps/transfer/ibc_module.go

@@ -347,9 +347,6 @@ func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpar
 func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) {
 }
 
-// OnChanUpgradeRestore implements the IBCModule interface
-func (IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {}
-
 // UnmarshalPacketData attempts to unmarshal the provided packet data bytes
 // into a FungibleTokenPacketData. This function implements the optional
 // PacketDataUnmarshaler interface required for ADR 008 support.