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

Allow host_connection_id in version metadata to be empty on ChanOpenInit #5533

Merged
merged 63 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 58 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
2f33154
remove host connection ID in default metadata
GNaD13 Jan 7, 2024
e535ddb
allow empty host connection id in validateConnectionParams
GNaD13 Jan 7, 2024
670ff0d
add host connection id to the metadata on host open try
GNaD13 Jan 7, 2024
bc119a7
nit
GNaD13 Jan 7, 2024
d0f0831
add test init with empty host connection ID
GNaD13 Jan 7, 2024
a37e7f1
add test host
GNaD13 Jan 7, 2024
0e377ef
correct test
GNaD13 Jan 7, 2024
b9f280b
nit
GNaD13 Jan 7, 2024
c92c59b
lint
Jan 8, 2024
677865d
Merge branch 'main' into dang/host-connection-id
Jan 8, 2024
fc05c68
remove assert
GNaD13 Jan 8, 2024
4280d46
Merge remote-tracking branch 'decent/dang/host-connection-id' into da…
GNaD13 Jan 8, 2024
1162019
TestOnChanOpenInit
GNaD13 Jan 9, 2024
237ade9
OnChanOpenTry
GNaD13 Jan 9, 2024
b53a2ff
TestOnChanUpgradeInit
GNaD13 Jan 9, 2024
0324206
TestOnChanUpgradeTry
GNaD13 Jan 9, 2024
b8e8fe2
TestOnChanUpgradeTry
GNaD13 Jan 9, 2024
a1a5e0f
remove check
GNaD13 Jan 9, 2024
e1d2f77
test
GNaD13 Jan 9, 2024
cbb39da
test
GNaD13 Jan 9, 2024
6d86f85
nit
GNaD13 Jan 9, 2024
840953f
Merge remote-tracking branch 'origin/main' into dang/host-connection-id
GNaD13 Jan 9, 2024
61f3fb5
nit
GNaD13 Jan 10, 2024
e222c4b
add check version
GNaD13 Jan 10, 2024
0b0b7d0
NewDefaultMetadataString
GNaD13 Jan 10, 2024
858954b
e2e
GNaD13 Jan 10, 2024
3b092a6
change func name and var place
GNaD13 Jan 17, 2024
ad3e344
callbacks
GNaD13 Jan 17, 2024
27d488f
revert icatypes.NewDefaultMetadataString
GNaD13 Jan 17, 2024
fd23ea2
add back NewDefaultMetadataString with hostConnectionID
GNaD13 Jan 17, 2024
ef00a21
nit
GNaD13 Jan 17, 2024
9abb309
nit
GNaD13 Jan 17, 2024
e7cce12
Merge remote-tracking branch 'origin/main' into dang/host-connection-id
GNaD13 Jan 18, 2024
5a6a68a
nit
GNaD13 Jan 18, 2024
cebec2b
remove unused code
GNaD13 Jan 18, 2024
56bbbc8
Merge branch 'main' into dang/host-connection-id
GNaD13 Jan 18, 2024
530ecd9
revert
GNaD13 Jan 19, 2024
553dd63
Merge remote-tracking branch 'decent/dang/host-connection-id' into da…
GNaD13 Jan 19, 2024
9ed3d4e
Merge branch 'main' into dang/host-connection-id
GNaD13 Jan 19, 2024
f2e58d9
Merge branch 'main' into dang/host-connection-id
DimitrisJim Jan 25, 2024
fca1943
Merge branch 'main' into dang/host-connection-id
Feb 1, 2024
de2ef57
address self-review comments
Feb 1, 2024
ef661c0
Merge branch 'cosmos:dang/host-connection-id' into dang/host-connecti…
Feb 1, 2024
6af728f
add import back
Feb 1, 2024
2903797
Merge branch 'cosmos:dang/host-connection-id' into dang/host-connecti…
Feb 1, 2024
cfd440a
some more refactoring
Feb 1, 2024
ceb0205
Merge branch 'cosmos:dang/host-connection-id' into dang/host-connecti…
Feb 1, 2024
61b3505
more cleanup
Feb 1, 2024
7cbf91e
Merge branch 'cosmos:dang/host-connection-id' into dang/host-connecti…
Feb 1, 2024
8f381a5
remove unused test
GNaD13 Feb 2, 2024
f9a8e4c
change set version func
GNaD13 Feb 2, 2024
2287133
lint
GNaD13 Feb 2, 2024
fbfbb0f
Merge branch 'main' into dang/host-connection-id
GNaD13 Feb 3, 2024
99b4443
Merge branch 'main' into dang/host-connection-id
GNaD13 Feb 9, 2024
e8501ec
Merge branch 'main' into dang/host-connection-id
GNaD13 Mar 5, 2024
e23cf85
still set host connection ID in our controller implementation
Mar 7, 2024
abcff64
remove empty lines
Mar 7, 2024
5b5b609
remove unnecessary line
Mar 8, 2024
883d795
add changelog
Mar 8, 2024
c2b7850
Merge branch 'main' into dang/host-connection-id
Mar 8, 2024
3606daa
Merge branch 'main' into dang/host-connection-id
Mar 8, 2024
59fdf13
Merge branch 'main' into dang/host-connection-id
Mar 12, 2024
d7cfafd
Merge branch 'main' into dang/host-connection-id
Mar 12, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,7 @@ var (
TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress)

// TestVersion defines a reusable interchainaccounts version string for testing purposes
TestVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{
Version: icatypes.Version,
ControllerConnectionId: ibctesting.FirstConnectionID,
HostConnectionId: ibctesting.FirstConnectionID,
Encoding: icatypes.EncodingProtobuf,
TxType: icatypes.TxTypeSDKMultiMsg,
}))
TestVersion = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
)

type InterchainAccountsTestSuite struct {
Expand Down Expand Up @@ -244,7 +238,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
)

if tc.expPass {
suite.Require().Equal(icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID), version)
suite.Require().Equal(TestVersion, version)
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
Expand Down Expand Up @@ -842,8 +836,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
err := RegisterInterchainAccount(path.EndpointA, TestOwnerAddress)
suite.Require().NoError(err)

metadata := icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
version = icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)

tc.malleate() // malleate mutates test data

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ const (

func (suite *KeeperTestSuite) TestOnChanOpenInit() {
var (
channel *channeltypes.Channel
path *ibctesting.Path
chanCap *capabilitytypes.Capability
metadata icatypes.Metadata
channel *channeltypes.Channel
path *ibctesting.Path
chanCap *capabilitytypes.Capability
metadata icatypes.Metadata
expectedVersion string
)

testCases := []struct {
Expand Down Expand Up @@ -54,6 +55,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
"success: empty channel version returns default metadata JSON string",
func() {
channel.Version = ""
expectedVersion = icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
},
nil,
},
Expand Down Expand Up @@ -275,6 +277,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata)
suite.Require().NoError(err)

expectedVersion = string(versionBytes)

counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
channel = &channeltypes.Channel{
State: channeltypes.INIT,
Expand All @@ -296,7 +300,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
expPass := tc.expError == nil
if expPass {
suite.Require().NoError(err)
suite.Require().Equal(string(versionBytes), version)
suite.Require().Equal(expectedVersion, version)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expError)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type MigrationsTestSuite struct {
}

func (suite *MigrationsTestSuite) SetupTest() {
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)

suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)

suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
Expand All @@ -38,8 +40,8 @@ func (suite *MigrationsTestSuite) SetupTest() {
suite.path.EndpointB.ChannelConfig.PortID = icatypes.HostPortID
suite.path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED
suite.path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED
suite.path.EndpointA.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
suite.path.EndpointB.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
suite.path.EndpointA.ChannelConfig.Version = version
suite.path.EndpointB.ChannelConfig.Version = version
}

func (suite *MigrationsTestSuite) SetupPath() error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func (k Keeper) OnChanOpenTry(
return "", err
}

// set here the HostConnectionId in case the controller did not set it
metadata.HostConnectionId = connectionHops[0]

if err = icatypes.ValidateHostMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
return "", err
}
Expand Down
48 changes: 30 additions & 18 deletions modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
GNaD13 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
suite.Require().NoError(err)

suite.openAndCloseChannel(path)
}, true,
},
true,
},
{
"success - reopening account with new address",
Expand All @@ -90,7 +91,20 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
// assert interchain account address mapping was deleted
_, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
suite.Require().False(found)
}, true,
},
true,
},
{
"success - empty host connection ID",
func() {
metadata.HostConnectionId = ""

versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata)
suite.Require().NoError(err)

path.EndpointA.ChannelConfig.Version = string(versionBytes)
},
true,
},
{
"success - previous metadata is different",
Expand Down Expand Up @@ -128,7 +142,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {

acc := suite.chainB.GetSimApp().AccountKeeper.GetAccount(suite.chainB.GetContext(), sdk.MustAccAddressFromBech32(addr))
suite.chainB.GetSimApp().AccountKeeper.RemoveAccount(suite.chainB.GetContext(), acc)
}, false,
},
false,
},
{
"reopening account fails - existing account is not interchain account type",
Expand All @@ -152,7 +167,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {

// overwrite existing account with only base account type, not intercahin account type
suite.chainB.GetSimApp().AccountKeeper.SetAccount(suite.chainB.GetContext(), icaAcc.BaseAccount)
}, false,
},
false,
},
{
"account already exists",
Expand Down Expand Up @@ -222,18 +238,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
},
false,
},
{
"invalid host connection ID",
func() {
metadata.HostConnectionId = "invalid-connnection-id"

versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata)
suite.Require().NoError(err)

path.EndpointA.ChannelConfig.Version = string(versionBytes)
},
false,
},
{
"invalid counterparty version",
func() {
Expand Down Expand Up @@ -264,7 +268,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {

// set the active channelID in state
suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID)
}, false,
},
false,
},
{
"channel is already active (FLUSHING state)",
Expand Down Expand Up @@ -306,6 +311,8 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata)
suite.Require().NoError(err)

expectedMetadata := metadata

counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
channel = &channeltypes.Channel{
State: channeltypes.TRYOPEN,
Expand Down Expand Up @@ -336,6 +343,12 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
// Check if account is created
interchainAccount := suite.chainB.GetSimApp().AccountKeeper.GetAccount(suite.chainB.GetContext(), interchainAccAddr)
suite.Require().Equal(interchainAccount.GetAddress().String(), storedAddr)

expectedMetadata.Address = storedAddr
expectedVersionBytes, err := icatypes.ModuleCdc.MarshalJSON(&expectedMetadata)
suite.Require().NoError(err)

suite.Require().Equal(string(expectedVersionBytes), version)
} else {
suite.Require().Error(err)
suite.Require().Equal("", version)
Expand Down Expand Up @@ -577,7 +590,6 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
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

Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/types/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewMetadata(version, controllerConnectionID, hostConnectionID, accAddress,
}

// NewDefaultMetadata creates and returns a new ICS27 Metadata instance containing the default ICS27 Metadata values
// with the provided controller and host connection identifiers
// with the provided controller and host connection identifiers. The host connection identifier may be an empty string.
func NewDefaultMetadata(controllerConnectionID, hostConnectionID string) Metadata {
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
metadata := Metadata{
ControllerConnectionId: controllerConnectionID,
Expand All @@ -47,7 +47,7 @@ func NewDefaultMetadata(controllerConnectionID, hostConnectionID string) Metadat
}

// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values
// with the provided controller and host connection identifiers
// with the provided controller and host connection identifiers. The host connection identifier may be an empty string.
func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string {
GNaD13 marked this conversation as resolved.
Show resolved Hide resolved
metadata := NewDefaultMetadata(controllerConnectionID, hostConnectionID)

Expand Down
Loading