From 474197726053040cd02676037dd6b2ba65b96470 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 21 Sep 2021 09:50:53 +0200 Subject: [PATCH 1/4] adding module account to interchain-accounts with associated changes --- .../keeper/grpc_query_test.go | 6 ++--- .../keeper/handshake.go | 4 +-- .../27-interchain-accounts/keeper/keeper.go | 27 +++++++++++++++++++ .../keeper/keeper_test.go | 9 +++++-- modules/apps/27-interchain-accounts/module.go | 6 +---- .../27-interchain-accounts/module_test.go | 7 ++++- .../27-interchain-accounts/types/account.go | 8 +++--- .../types/account_test.go | 2 +- .../types/expected_keepers.go | 6 +++-- 9 files changed, 55 insertions(+), 20 deletions(-) diff --git a/modules/apps/27-interchain-accounts/keeper/grpc_query_test.go b/modules/apps/27-interchain-accounts/keeper/grpc_query_test.go index c5ea7a3cd84..9387a24b92e 100644 --- a/modules/apps/27-interchain-accounts/keeper/grpc_query_test.go +++ b/modules/apps/27-interchain-accounts/keeper/grpc_query_test.go @@ -48,12 +48,12 @@ func (suite *KeeperTestSuite) TestQueryInterchainAccountAddress() { { "success", func() { - expAddr = authtypes.NewBaseAccountWithAddress(types.GenerateAddress("ics-27")).GetAddress().String() + expAddr = authtypes.NewBaseAccountWithAddress(types.GenerateAddress(TestAccAddress, TestPortID)).GetAddress().String() req = &types.QueryInterchainAccountAddressRequest{ - CounterpartyPortId: "ics-27", + CounterpartyPortId: TestPortID, } - suite.chainA.GetSimApp().ICAKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), "ics-27", expAddr) + suite.chainA.GetSimApp().ICAKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), TestPortID, expAddr) }, true, }, diff --git a/modules/apps/27-interchain-accounts/keeper/handshake.go b/modules/apps/27-interchain-accounts/keeper/handshake.go index bccd1be1a16..04ce58da40e 100644 --- a/modules/apps/27-interchain-accounts/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/keeper/handshake.go @@ -87,8 +87,8 @@ func (k Keeper) OnChanOpenTry( return err } - // Check to ensure that the version string contains the expected address generated from the Counterparty portID - accAddr := types.GenerateAddress(counterparty.PortId) + // Check to ensure that the version string contains the expected address generated from the Counterparty portID + accAddr := types.GenerateAddress(k.accountKeeper.GetModuleAddress(types.ModuleName), counterparty.PortId) parsedAddr := types.ParseAddressFromVersion(version) if parsedAddr != accAddr.String() { return sdkerrors.Wrapf(types.ErrInvalidAccountAddress, "version contains invalid account address: expected %s, got %s", parsedAddr, accAddr) diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go index d45fd9037aa..fb482694e12 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper.go @@ -7,11 +7,13 @@ import ( "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/tendermint/tendermint/libs/log" "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" + channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/modules/core/24-host" ) @@ -37,6 +39,12 @@ func NewKeeper( channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper, accountKeeper types.AccountKeeper, scopedKeeper capabilitykeeper.ScopedKeeper, msgRouter *baseapp.MsgServiceRouter, hook types.IBCAccountHooks, ) Keeper { + + // ensure ibc interchain accounts module account is set + if addr := accountKeeper.GetModuleAddress(types.ModuleName); addr == nil { + panic("the IBC interchain accounts module account has not been set") + } + return Keeper{ storeKey: key, cdc: cdc, @@ -163,3 +171,22 @@ func (k Keeper) SetInterchainAccountAddress(ctx sdk.Context, portID string, addr store := ctx.KVStore(k.storeKey) store.Set(types.KeyOwnerAccount(portID), []byte(address)) } + +// NegotiateAppVersion handles application version negotation for the IBC interchain accounts module +func (k Keeper) NegotiateAppVersion( + ctx sdk.Context, + order channeltypes.Order, + connectionID string, + portID string, + counterparty channeltypes.Counterparty, + proposedVersion string, +) (string, error) { + if proposedVersion != types.VersionPrefix { + return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: expected %s, got %s", types.VersionPrefix, proposedVersion) + } + + moduleAccAddr := k.accountKeeper.GetModuleAddress(types.ModuleName) + accAddr := types.GenerateAddress(moduleAccAddr, counterparty.PortId) + + return types.NewAppVersion(types.VersionPrefix, accAddr.String()), nil +} diff --git a/modules/apps/27-interchain-accounts/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/keeper/keeper_test.go index 0f7c12d8189..ac9f234d6ec 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper_test.go @@ -4,8 +4,10 @@ import ( "fmt" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/suite" + "github.com/tendermint/tendermint/crypto" "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" @@ -13,12 +15,15 @@ import ( ) var ( + // TestAccAddress defines a resuable bech32 address for testing purposes + // TODO: update crypto.AddressHash() when sdk uses address.Module() + TestAccAddress = types.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(types.ModuleName))), TestPortID) // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" // TestPortID defines a resuable port identifier for testing purposes TestPortID = fmt.Sprintf("ics-27-0-0-%s", TestOwnerAddress) // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(TestPortID).String()) + TestVersion = types.NewAppVersion(types.VersionPrefix, TestAccAddress.String()) ) type KeeperTestSuite struct { @@ -118,7 +123,7 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() { suite.Require().NoError(err) counterpartyPortID := path.EndpointA.ChannelConfig.PortID - expectedAddr := authtypes.NewBaseAccountWithAddress(types.GenerateAddress(counterpartyPortID)).GetAddress() + expectedAddr := authtypes.NewBaseAccountWithAddress(types.GenerateAddress(suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName), counterpartyPortID)).GetAddress() retrievedAddr, found := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), counterpartyPortID) suite.Require().True(found) diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go index efa808f8d01..efcafe808a2 100644 --- a/modules/apps/27-interchain-accounts/module.go +++ b/modules/apps/27-interchain-accounts/module.go @@ -264,9 +264,5 @@ func (am AppModule) NegotiateAppVersion( counterparty channeltypes.Counterparty, proposedVersion string, ) (string, error) { - if proposedVersion != types.VersionPrefix { - return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "failed to negotiate app version: expected %s, got %s", types.VersionPrefix, proposedVersion) - } - - return types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(counterparty.PortId).String()), nil + return am.keeper.NegotiateAppVersion(ctx, order, connectionID, portID, counterparty, proposedVersion) } diff --git a/modules/apps/27-interchain-accounts/module_test.go b/modules/apps/27-interchain-accounts/module_test.go index fbfea84a189..45c2fd97bd2 100644 --- a/modules/apps/27-interchain-accounts/module_test.go +++ b/modules/apps/27-interchain-accounts/module_test.go @@ -5,19 +5,24 @@ import ( "testing" "github.com/stretchr/testify/suite" + "github.com/tendermint/tendermint/crypto" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/testing" ) var ( + // TestAccAddress defines a resuable bech32 address for testing purposes + // TODO: update crypto.AddressHash() when sdk uses address.Module() + TestAccAddress = types.GenerateAddress(sdk.AccAddress(crypto.AddressHash([]byte(types.ModuleName))), TestPortID) // TestOwnerAddress defines a reusable bech32 address for testing purposes TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs" // TestPortID defines a resuable port identifier for testing purposes TestPortID = fmt.Sprintf("ics-27-0-0-%s", TestOwnerAddress) // TestVersion defines a resuable interchainaccounts version string for testing purposes - TestVersion = types.NewAppVersion(types.VersionPrefix, types.GenerateAddress(TestPortID).String()) + TestVersion = types.NewAppVersion(types.VersionPrefix, TestAccAddress.String()) ) type InterchainAccountsTestSuite struct { diff --git a/modules/apps/27-interchain-accounts/types/account.go b/modules/apps/27-interchain-accounts/types/account.go index 7948e2e6afa..82883411228 100644 --- a/modules/apps/27-interchain-accounts/types/account.go +++ b/modules/apps/27-interchain-accounts/types/account.go @@ -7,9 +7,9 @@ import ( crypto "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/tendermint/tendermint/crypto/tmhash" yaml "gopkg.in/yaml.v2" connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types" @@ -19,9 +19,9 @@ const ( ICAPrefix string = "ics-27" ) -// GenerateAddress returns an sdk.AccAddress using the provided port identifier -func GenerateAddress(portID string) sdk.AccAddress { - return sdk.AccAddress(tmhash.SumTruncated([]byte(portID))) +// GenerateAddress returns an sdk.AccAddress derived using the provided module account address and port identifier +func GenerateAddress(moduleAccAddr sdk.AccAddress, portID string) sdk.AccAddress { + return sdk.AccAddress(address.Derive(moduleAccAddr, []byte(portID))) } // ParseAddressFromVersion trims the interchainaccounts version prefix and returns the associated account address diff --git a/modules/apps/27-interchain-accounts/types/account_test.go b/modules/apps/27-interchain-accounts/types/account_test.go index 8f3bbc1126b..3d07ecd35a6 100644 --- a/modules/apps/27-interchain-accounts/types/account_test.go +++ b/modules/apps/27-interchain-accounts/types/account_test.go @@ -37,7 +37,7 @@ func TestTypesTestSuite(t *testing.T) { } func (suite *TypesTestSuite) TestGenerateAddress() { - addr := types.GenerateAddress("test-port-id") + addr := types.GenerateAddress([]byte{}, "test-port-id") accAddr, err := sdk.AccAddressFromBech32(addr.String()) suite.Require().NoError(err, "TestGenerateAddress failed") diff --git a/modules/apps/27-interchain-accounts/types/expected_keepers.go b/modules/apps/27-interchain-accounts/types/expected_keepers.go index 448a0b3fcaa..282ab2c6dd5 100644 --- a/modules/apps/27-interchain-accounts/types/expected_keepers.go +++ b/modules/apps/27-interchain-accounts/types/expected_keepers.go @@ -15,10 +15,12 @@ type Router interface { // AccountKeeper defines the contract required for account APIs. type AccountKeeper interface { - SetAccount(ctx sdk.Context, acc authtypes.AccountI) - GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI NewAccount(ctx sdk.Context, acc authtypes.AccountI) authtypes.AccountI NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI + SetAccount(ctx sdk.Context, acc authtypes.AccountI) + GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI + GetModuleAddress(name string) sdk.AccAddress } // ChannelKeeper defines the expected IBC channel keeper From 91eacb5ab15490956d94f01a2f9ae2b1a64bf170 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 21 Sep 2021 09:51:20 +0200 Subject: [PATCH 2/4] configuring ica module account in simapp --- testing/simapp/app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 6592c6ed424..3d77898f98d 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -147,6 +147,7 @@ var ( stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking}, govtypes.ModuleName: {authtypes.Burner}, ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, + icatypes.ModuleName: nil, } ) From f92cde7c5eaf364126a0867a7b0c596279fb7019 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 24 Sep 2021 13:06:57 +0200 Subject: [PATCH 3/4] Update modules/apps/27-interchain-accounts/keeper/keeper.go Co-authored-by: Sean King --- modules/apps/27-interchain-accounts/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go index 78b16e6899e..832755817b5 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper.go @@ -42,7 +42,7 @@ func NewKeeper( // ensure ibc interchain accounts module account is set if addr := accountKeeper.GetModuleAddress(types.ModuleName); addr == nil { - panic("the IBC interchain accounts module account has not been set") + panic("the Interchain Accounts module account has not been set") } return Keeper{ From 391ecd02c8a607a877a9052a1845770f9c450cc6 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 24 Sep 2021 13:39:27 +0200 Subject: [PATCH 4/4] updating godoc and import alias --- modules/apps/27-interchain-accounts/types/account.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/apps/27-interchain-accounts/types/account.go b/modules/apps/27-interchain-accounts/types/account.go index bc6989276f5..da2011146a3 100644 --- a/modules/apps/27-interchain-accounts/types/account.go +++ b/modules/apps/27-interchain-accounts/types/account.go @@ -7,7 +7,7 @@ import ( crypto "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" + sdkaddress "github.com/cosmos/cosmos-sdk/types/address" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" yaml "gopkg.in/yaml.v2" @@ -15,9 +15,10 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v2/modules/core/03-connection/types" ) -// GenerateAddress returns an sdk.AccAddress derived using the provided module account address and port identifier +// GenerateAddress returns an sdk.AccAddress derived using the provided module account address and port identifier. +// The sdk.AccAddress returned is a sub-address of the module account, using the controller chain's port identifier as the derivation key func GenerateAddress(moduleAccAddr sdk.AccAddress, portID string) sdk.AccAddress { - return sdk.AccAddress(address.Derive(moduleAccAddr, []byte(portID))) + return sdk.AccAddress(sdkaddress.Derive(moduleAccAddr, []byte(portID))) } // ParseAddressFromVersion trims the interchainaccounts version prefix and returns the associated account address