Skip to content

Commit

Permalink
still set host connection ID in our controller implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
Carlos Rodriguez committed Mar 7, 2024
1 parent e8501ec commit e23cf85
Show file tree
Hide file tree
Showing 19 changed files with 48 additions and 63 deletions.
16 changes: 0 additions & 16 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,6 @@ There are four sections based on the four potential user groups of this document

## IBC Apps

### ICS27 - Interchain Accounts

- The signature of function `NewDefaultMetadata` has changed:

```diff
- func NewDefaultMetadata(controllerConnectionID, hostConnectionID string) Metadata
+ func NewDefaultMetadata(controllerConnectionID string) Metadata
```

- The signature of function `NewDefaultMetadataString` has changed:

```diff
- func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string)
+ func NewDefaultMetadataString(controllerConnectionID string) string
```

### API removals

The `exported.ChannelI` and `exported.CounterpartyChannelI` interfaces have been removed. Please use the concrete types.
Expand Down
8 changes: 4 additions & 4 deletions e2e/tests/interchain_accounts/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan

t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, order)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
Expand Down Expand Up @@ -172,7 +172,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF

t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
Expand Down Expand Up @@ -270,7 +270,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
t.Run("register interchain account", func(t *testing.T) {
var err error
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
Expand Down Expand Up @@ -369,7 +369,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
// on an ordered channel
t.Run("register interchain account", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/interchain_accounts/gov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
s.Require().NotNil(govModuleAddress)

t.Run("execute proposal for MsgRegisterInterchainAccount", func(t *testing.T) {
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version, channeltypes.ORDERED)

s.ExecuteAndPassGovV1Proposal(ctx, msgRegisterAccount, chainA, controllerAccount)
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/interchain_accounts/groups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat

t.Run("submit proposal for MsgRegisterInterchainAccount", func(t *testing.T) {
groupPolicyAddr = s.QueryGroupPolicyAddress(ctx, chainA)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), channeltypes.ORDERED)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), channeltypes.ORDERED)

msgSubmitProposal, err := grouptypes.NewMsgSubmitProposal(groupPolicyAddr, []string{chainAAddress}, []sdk.Msg{msgRegisterAccount}, DefaultMetadata, grouptypes.Exec_EXEC_UNSPECIFIED, "e2e groups proposal: for MsgRegisterInterchainAccount", "e2e groups proposal: for MsgRegisterInterchainAccount")
s.Require().NoError(err)
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/interchain_accounts/localhost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_Localhost(

s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA), "failed to wait for blocks")

version := icatypes.NewDefaultMetadataStringWithHostConnectionID(exported.LocalhostConnectionID, exported.LocalhostConnectionID)
version := icatypes.NewDefaultMetadataString(exported.LocalhostConnectionID, exported.LocalhostConnectionID)
controllerPortID, err := icatypes.NewControllerPortID(userAWallet.FormattedAddress())
s.Require().NoError(err)

Expand Down Expand Up @@ -214,7 +214,7 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_ReopenChan

s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA), "failed to wait for blocks")

version := icatypes.NewDefaultMetadataStringWithHostConnectionID(exported.LocalhostConnectionID, exported.LocalhostConnectionID)
version := icatypes.NewDefaultMetadataString(exported.LocalhostConnectionID, exported.LocalhostConnectionID)
controllerPortID, err := icatypes.NewControllerPortID(userAWallet.FormattedAddress())
s.Require().NoError(err)

Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/interchain_accounts/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (s *InterchainAccountsParamsTestSuite) TestControllerEnabledParam() {

t.Run("ensure that broadcasting a MsgRegisterInterchainAccount fails", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/interchain_accounts/upgrades_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (s *InterchainAccountsChannelUpgradesTestSuite) TestMsgSendTx_SuccessfulTra
t.Run("register interchain account", func(t *testing.T) {
var err error
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
Expand Down Expand Up @@ -249,7 +249,7 @@ func (s *InterchainAccountsChannelUpgradesTestSuite) TestChannelUpgrade_ICAChann
t.Run("register interchain account", func(t *testing.T) {
var err error
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)
txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
s.AssertTxSuccess(txResp)
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/upgrades/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *GenesisTestSuite) TestIBCGenesis() {

t.Run("ics27: broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var (
TestPortID, _ = icatypes.NewControllerPortID(TestOwnerAddress)

// TestVersion defines a reusable interchainaccounts version string for testing purposes
TestVersion = icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
TestVersion = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
)

type InterchainAccountsTestSuite struct {
Expand Down Expand Up @@ -836,7 +836,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
err := RegisterInterchainAccount(path.EndpointA, TestOwnerAddress)
suite.Require().NoError(err)

version = icatypes.NewDefaultMetadataStringWithHostConnectionID(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
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 @@ -44,18 +44,23 @@ func (k Keeper) OnChanOpenInit(
)

if strings.TrimSpace(version) == "" {
metadata = icatypes.NewDefaultMetadata(connectionHops[0])
} else {
metadata, err = icatypes.MetadataFromVersion(version)
connection, err := k.channelKeeper.GetConnection(ctx, connectionHops[0])
if err != nil {
return "", err
}

if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
metadata = icatypes.NewDefaultMetadata(connectionHops[0], connection.Counterparty.ConnectionId)
} else {
metadata, err = icatypes.MetadataFromVersion(version)
if err != nil {
return "", err
}
}

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

activeChannelID, found := k.GetActiveChannelID(ctx, connectionHops[0], portID)
if found {
channel, found := k.channelKeeper.GetChannel(ctx, portID, activeChannelID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
"success: empty channel version returns default metadata JSON string",
func() {
channel.Version = ""
expectedVersion = icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID)
expectedVersion = icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
},
nil,
},
Expand Down Expand Up @@ -172,6 +172,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
},
connectiontypes.ErrConnectionNotFound,
},
{
"connection not found with default empty channel version",
func() {
channel.ConnectionHops = []string{"connection-10"}
channel.Version = ""
},
connectiontypes.ErrConnectionNotFound,
},
{
"invalid controller connection ID",
func() {
Expand Down Expand Up @@ -648,8 +656,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
suite.Require().NoError(err)

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

Expand Down Expand Up @@ -814,9 +821,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
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)
// add host connection id to metadata
metadata.HostConnectionId = path.EndpointB.ConnectionID
metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
// use the same address as the previous metadata.
metadata.Address = currentMetadata.Address

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type MigrationsTestSuite struct {
}

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestMsgRegisterInterchainAccountValidateBasic(t *testing.T) {
func() {
feeMetadata := feetypes.Metadata{
FeeVersion: feetypes.Version,
AppVersion: icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID),
AppVersion: icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID),
}

bz := feetypes.ModuleCdc.MustMarshalJSON(&feeMetadata)
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestMsgRegisterInterchainAccountValidateBasic(t *testing.T) {
msg = types.NewMsgRegisterInterchainAccount(
ibctesting.FirstConnectionID,
ibctesting.TestAccAddress,
icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID),
icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID),
channeltypes.ORDERED,
)

Expand Down
3 changes: 1 addition & 2 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
suite.Require().True(found)

metadata := icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID)
metadata.HostConnectionId = path.EndpointB.ConnectionID
metadata := icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
metadata.Address = interchainAccountAddr
metadata.Encoding = icatypes.EncodingProto3JSON // this is the actual change to the version
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (k Keeper) OnChanOpenTry(
return "", err
}

// set here the HostConnectionId because the controller did not set it
// 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
suite.Require().NoError(err)

order = channeltypes.ORDERED
metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID)
metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
metadata.HostConnectionId = path.EndpointB.ConnectionID
// use the same address as the previous metadata.
metadata.Address = currentMetadata.Address
Expand Down
20 changes: 6 additions & 14 deletions modules/apps/27-interchain-accounts/types/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ 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 connection identifier
func NewDefaultMetadata(controllerConnectionID string) Metadata {
// with the provided controller and host connection identifiers. The host connection identifier may be an empty string.
func NewDefaultMetadata(controllerConnectionID, hostConnectionID string) Metadata {
metadata := Metadata{
ControllerConnectionId: controllerConnectionID,
HostConnectionId: "",
HostConnectionId: hostConnectionID,
Encoding: EncodingProtobuf,
TxType: TxTypeSDKMultiMsg,
Version: Version,
Expand All @@ -47,17 +47,9 @@ func NewDefaultMetadata(controllerConnectionID string) Metadata {
}

// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values
// with the provided controller connection identifier and an empty host connection identifier
func NewDefaultMetadataString(controllerConnectionID string) string {
metadata := NewDefaultMetadata(controllerConnectionID)

return string(ModuleCdc.MustMarshalJSON(&metadata))
}

// NewDefaultMetadataStringWithHostConnectionID creates and returns a new JSON encoded version string containing the default ICS27 Metadata values
// with the provided controller and host connection identifiers
func NewDefaultMetadataStringWithHostConnectionID(controllerConnectionID, hostConnectionID string) string {
metadata := NewMetadata(Version, controllerConnectionID, hostConnectionID, "", EncodingProtobuf, TxTypeSDKMultiMsg)
// with the provided controller and host connection identifiers. The host connection identifier may be an empty string.
func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string {
metadata := NewDefaultMetadata(controllerConnectionID, hostConnectionID)

return string(ModuleCdc.MustMarshalJSON(&metadata))
}
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/ica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var (
defaultPortID, _ = icatypes.NewControllerPortID(defaultOwnerAddress)

// defaultICAVersion defines a reusable interchainaccounts version string for testing purposes
defaultICAVersion = icatypes.NewDefaultMetadataStringWithHostConnectionID(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
defaultICAVersion = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
)

// NewIncentivizedICAPath creates and returns a new ibctesting path configured for a fee enabled interchain accounts channel
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/callbacks/callbacks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (s *CallbacksTestSuite) SetupICATest() string {

icaOwner := s.chainA.SenderAccount.GetAddress().String()
// ICAVersion defines a interchain accounts version string
icaVersion := icatypes.NewDefaultMetadataStringWithHostConnectionID(s.path.EndpointA.ConnectionID, s.path.EndpointB.ConnectionID)
icaVersion := icatypes.NewDefaultMetadataString(s.path.EndpointA.ConnectionID, s.path.EndpointB.ConnectionID)
icaControllerPortID, err := icatypes.NewControllerPortID(icaOwner)
s.Require().NoError(err)

Expand Down

0 comments on commit e23cf85

Please sign in to comment.