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.

Sign up for GitHub

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

Jump to bottom

feat: module account address derivation #428

Merged
merged 6 commits into from
Sep 24, 2021
Merged

feat: module account address derivation #428

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4741977
adding module account to interchain-accounts with associated changes
damiannolan Sep 21, 2021
91eacb5
configuring ica module account in simapp
damiannolan Sep 21, 2021
9f22b34
Merge branch 'interchain-accounts' into damian/303-module-account-der…
damiannolan Sep 24, 2021
f92cde7
Update modules/apps/27-interchain-accounts/keeper/keeper.go
damiannolan Sep 24, 2021
391ecd0
updating godoc and import alias
damiannolan Sep 24, 2021
ba8b17a
Merge branch 'interchain-accounts' into damian/303-module-account-der…
damiannolan Sep 24, 2021
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
6 changes: 3 additions & 3 deletions modules/apps/27-interchain-accounts/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/keeper/handshake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions modules/apps/27-interchain-accounts/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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")
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

return Keeper{
storeKey: key,
cdc: cdc,
Expand Down Expand Up @@ -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(
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the implementation of this from AppModule to the Keeper in order to gain access to the accountKeeper without having to expose it.
AppModule now just invokes this func on the Keeper.

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
}
9 changes: 7 additions & 2 deletions modules/apps/27-interchain-accounts/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,26 @@ 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"
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)
Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably the sdk will eventually be using address.Module() from ADR-28 here: https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/types/account.go#L164

So we'll need to replace crypto.AddressHash() when that's added

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we open an issue to track this? Should we open an issue on the SDK?

Copy link
Member Author

@damiannolan damiannolan Sep 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, I can open an issue on the SDK for this. I'll cc you

edit: cosmos/cosmos-sdk#10225

// 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 {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 1 addition & 5 deletions modules/apps/27-interchain-accounts/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 6 additions & 1 deletion modules/apps/27-interchain-accounts/module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions modules/apps/27-interchain-accounts/types/account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
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"
Expand All @@ -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
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
func GenerateAddress(moduleAccAddr sdk.AccAddress, portID string) sdk.AccAddress {
return sdk.AccAddress(address.Derive(moduleAccAddr, []byte(portID)))
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}

// ParseAddressFromVersion trims the interchainaccounts version prefix and returns the associated account address
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/types/account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 4 additions & 2 deletions modules/apps/27-interchain-accounts/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions testing/simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ var (
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking},
govtypes.ModuleName: {authtypes.Burner},
ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner},
icatypes.ModuleName: nil,
}
)

Expand Down