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 10 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
40 changes: 40 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 @@ -136,3 +137,42 @@ 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.ParseMedataFromString(version)
if err != nil {
return "", err
}

channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
if !found {
return "", errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
}

currentMetadata, err := icatypes.ParseMedataFromString(channel.Version)
if err != nil {
return "", err
}
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

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, "address cannot be changed")
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}

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

metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
return string(metadataBz), 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,106 @@ 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: invalid metadata",
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
malleate: func() {
metadata.Address = TestOwnerAddress
},
expError: icatypes.ErrInvalidAccountAddress,
},
{
name: "failure: change connection id",
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
malleate: func() {
metadata.ControllerConnectionId = differentConnectionID
},
expError: connectiontypes.ErrInvalidConnection,
},
{
name: "failure: change host connection id",
malleate: func() {
metadata.HostConnectionId = differentConnectionID
},
expError: connectiontypes.ErrInvalidConnection,
},
{
name: "failure: invalid metadata string",
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
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)

channel := path.EndpointA.GetChannel()

currentMetadata, err := icatypes.ParseMedataFromString(channel.Version)
suite.Require().NoError(err)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

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)
}
})
}
}
9 changes: 9 additions & 0 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))
}

// ParseMedataFromString parses Metadata from a json encoded version string.
func ParseMedataFromString(versionString string) (Metadata, error) {
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
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
2 changes: 1 addition & 1 deletion modules/capability/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func NewAppModuleBasic(cdc codec.Codec) AppModuleBasic {
}

// Name returns the capability module's name.
func (am AppModuleBasic) Name() string {
func (AppModuleBasic) Name() string {
return types.ModuleName
}

Expand Down
4 changes: 2 additions & 2 deletions scripts/go-lint-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ lint_module() {
local root="$1"
shift
cd "$(dirname "$root")" &&
echo "linting $(grep "^module" go.mod) [$(date -Iseconds -u)]" &&
echo "linting $(grep "^module" go.mod) [$(date -u +"%Y-%m-%dT%H:%M:%S")]" &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to the version on main which works on macOS

golangci-lint run ./... -c "${REPO_ROOT}/.golangci.yml" "$@"
}
export -f lint_module

find "${REPO_ROOT}" -type f -name go.mod -print0 |
xargs -0 -I{} bash -c 'lint_module "$@"' _ {} "$@"
xargs -0 -I{} bash -c 'lint_module "$@"' _ {} "$@"
Loading