Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement OnChanUpgradeInit on Controller Chain for interchain-accounts #5141

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7372125
chore: adding controller implementation for OnChanUpgradeInit
chatton Nov 20, 2023
4427f1d
chore: happy path test passing
chatton Nov 20, 2023
978a7a2
chore: adding fail case
chatton Nov 20, 2023
925392f
chore: adding additional test cases
chatton Nov 21, 2023
fa5e349
chore: fix linting
chatton Nov 21, 2023
32dba02
chore: improving errors
chatton Nov 21, 2023
292529e
chore: refactor to use test keeper function directly
chatton Nov 21, 2023
eb76314
chore: add check for enabled controller module
chatton Nov 21, 2023
236f2d9
chore: call into middleware if provided
chatton Nov 21, 2023
e63e91a
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 22, 2023
bc477b9
chore: addressing PR feedback
chatton Nov 29, 2023
8ba8fe9
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 29, 2023
3a22d1c
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 29, 2023
024c602
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 29, 2023
83c2f8e
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 30, 2023
46a5b5a
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
chatton Nov 30, 2023
c8c6c6b
revert change in godoc of GetConnectionID
crodriguezvega Dec 2, 2023
21773db
fix: typo in MetadataFromVersion func
damiannolan Dec 5, 2023
19d47a9
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
damiannolan Dec 5, 2023
702634f
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
damiannolan Dec 5, 2023
f1c10fd
chore: rm duplicate func
damiannolan Dec 5, 2023
2bf3bd9
Merge branch '04-channel-upgrades' into cian/issue#4512-implement-upg…
damiannolan Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,26 @@ func (im IBCMiddleware) OnTimeoutPacket(
}

// OnChanUpgradeInit implements the IBCModule interface
func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
return icatypes.Version, nil
func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
if !im.keeper.GetParams(ctx).ControllerEnabled {
return "", types.ErrControllerSubModuleDisabled
}

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

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

if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
return im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, version)
}

return version, nil
}

// OnChanUpgradeTry implements the IBCModule interface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,19 @@ package keeper
This file is to allow for unexported functions and fields to be accessible to the testing package.
*/

import porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
import (
sdk "github.com/cosmos/cosmos-sdk/types"

icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
)

// GetICS4Wrapper is a getter for the keeper's ICS4Wrapper.
func (k *Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper {
return k.ics4Wrapper
}

// GetAppMetadata is a wrapper around getAppMetadata to allow the function to be directly called in tests.
func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
return k.getAppMetadata(ctx, portID, channelID)
}
34 changes: 34 additions & 0 deletions modules/apps/27-interchain-accounts/controller/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
)

Expand Down Expand Up @@ -139,3 +140,36 @@ func (Keeper) OnChanCloseConfirm(
) error {
return nil
}

// OnChanUpgradeInit performs the upgrade init step of the channel upgrade handshake.
func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, connectionHops []string, version string) (string, error) {
if strings.TrimSpace(version) == "" {
return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty")
}

metadata, err := icatypes.MetadataFromVersion(version)
if err != nil {
return "", err
}

currentMetadata, err := k.getAppMetadata(ctx, portID, channelID)
if err != nil {
return "", err
}

if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
return "", errorsmod.Wrap(err, "invalid metadata")
}

// the interchain account address on the host chain
// must remain the same after the upgrade.
if currentMetadata.Address != metadata.Address {
return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "interchain account address cannot be changed")
}

if currentMetadata.ControllerConnectionId != connectionHops[0] {
return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change")
}

return version, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@ package keeper_test
import (
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v8/testing"
)

const (
differentConnectionID = "connection-100"
)

func (suite *KeeperTestSuite) TestOnChanOpenInit() {
var (
channel *channeltypes.Channel
Expand Down Expand Up @@ -471,3 +476,104 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() {
})
}
}

func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
var (
path *ibctesting.Path
metadata icatypes.Metadata
)

testCases := []struct {
name string
malleate func()
expError error
}{
{
"success",
func() {},
nil,
},
{
name: "failure: change ICA address",
malleate: func() {
metadata.Address = TestOwnerAddress
},
expError: icatypes.ErrInvalidAccountAddress,
},
{
name: "failure: change controller connection id",
malleate: func() {
metadata.ControllerConnectionId = differentConnectionID
},
expError: connectiontypes.ErrInvalidConnection,
},
{
name: "failure: change host connection id",
malleate: func() {
metadata.HostConnectionId = differentConnectionID
},
expError: connectiontypes.ErrInvalidConnection,
},
{
name: "failure: cannot decode version string",
malleate: func() {
channel := path.EndpointA.GetChannel()
channel.Version = "invalid-metadata-string"
path.EndpointA.SetChannel(channel)
},
expError: icatypes.ErrUnknownDataType,
},
{
name: "failure: invalid connection hops",
malleate: func() {
path.EndpointA.ConnectionID = differentConnectionID
},
expError: connectiontypes.ErrConnectionNotFound,
},
}

for _, tc := range testCases {
tc := tc

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

path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

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

currentMetadata, err := suite.chainA.GetSimApp().ICAControllerKeeper.GetAppMetadata(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().NoError(err)

metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
// use the same address as the previous metadata.
metadata.Address = currentMetadata.Address

// this is the actual change to the version.
metadata.Encoding = icatypes.EncodingProto3JSON

tc.malleate() // malleate mutates test data

version := string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))

upgradeVersion, err := path.EndpointA.Chain.GetSimApp().ICAControllerKeeper.OnChanUpgradeInit(
path.EndpointA.Chain.GetContext(),
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ChannelID,
[]string{path.EndpointA.ConnectionID},
version,
)

expPass := tc.expError == nil

if expPass {
suite.Require().NoError(err)
suite.Require().Equal(upgradeVersion, version)
} else {
suite.Require().ErrorIs(err, tc.expError)
}
})
}
}
11 changes: 11 additions & 0 deletions modules/apps/27-interchain-accounts/controller/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
"github.com/cosmos/ibc-go/v8/modules/core/exported"
)

Expand Down Expand Up @@ -280,6 +281,16 @@ func (k Keeper) GetAuthority() string {
return k.authority
}

// getAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID
func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
appVersion, found := k.GetAppVersion(ctx, portID, channelID)
if !found {
return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID)
}

return icatypes.MetadataFromVersion(appVersion)
}

// GetParams returns the current ica/controller submodule parameters.
func (k Keeper) GetParams(ctx sdk.Context) types.Params {
store := ctx.KVStore(k.storeKey)
Expand Down
10 changes: 2 additions & 8 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,14 @@ func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string
return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID)
}

// GetAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID
// getAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID
func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) {
appVersion, found := k.GetAppVersion(ctx, portID, channelID)
if !found {
return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID)
}

metadata, err := icatypes.MetadataFromVersion(appVersion)
if err != nil {
// UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks
return icatypes.Metadata{}, err
}

return metadata, nil
return icatypes.MetadataFromVersion(appVersion)
}

// GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID
Expand Down
18 changes: 9 additions & 9 deletions modules/apps/27-interchain-accounts/types/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) s
return string(ModuleCdc.MustMarshalJSON(&metadata))
}

// MetadataFromVersion parses Metadata from a json encoded version string.
func MetadataFromVersion(versionString string) (Metadata, error) {
var metadata Metadata
if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil {
return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata")
}
return metadata, nil
}

// IsPreviousMetadataEqual compares a metadata to a previous version string set in a channel struct.
// It ensures all fields are equal except the Address string
func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool {
Expand Down Expand Up @@ -165,12 +174,3 @@ func validateConnectionParams(metadata Metadata, controllerConnectionID, hostCon

return nil
}

// MetadataFromVersion parses Metadata from a json encoded version string.
func MetadataFromVersion(versionString string) (Metadata, error) {
var metadata Metadata
if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil {
return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata")
}
return metadata, nil
}
Loading