Skip to content

Commit

Permalink
Merge branch 'main' of github.com:cosmos/ibc-go into colin/rm-channelI
Browse files Browse the repository at this point in the history
  • Loading branch information
colin-axner committed Jan 24, 2024
2 parents b199634 + 9faaff5 commit c28786f
Show file tree
Hide file tree
Showing 31 changed files with 776 additions and 487 deletions.
13 changes: 6 additions & 7 deletions docs/docs/01-ibc/06-channel-upgrades.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -152,8 +156,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
Expand Down Expand Up @@ -204,8 +206,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
Expand Down Expand Up @@ -241,7 +241,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.

Expand All @@ -251,8 +252,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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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
---

Expand Down
Original file line number Diff line number Diff line change
@@ -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
---

Expand Down
17 changes: 0 additions & 17 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1066,67 +1066,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down
3 changes: 0 additions & 3 deletions modules/apps/27-interchain-accounts/host/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
10 changes: 0 additions & 10 deletions modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
56 changes: 45 additions & 11 deletions modules/apps/29-fee/keeper/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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"
Expand All @@ -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) {
Expand All @@ -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
Expand Down
10 changes: 0 additions & 10 deletions modules/apps/callbacks/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit c28786f

Please sign in to comment.