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

feat: module account address derivation #428

Merged
merged 6 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (k Keeper) OnChanOpenTry(
}

// Check to ensure that the version string contains the expected address generated from the Counterparty portID
accAddr := types.GenerateAddress(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
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/v2/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v2/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 Interchain Accounts module account has not been set")
}

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
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/v2/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v2/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

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("%s-0-0-%s", types.VersionPrefix, 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,25 @@ import (
"fmt"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/suite"
"github.com/tendermint/tendermint/crypto"

"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v2/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("%s-0-0-%s", types.VersionPrefix, 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
11 changes: 6 additions & 5 deletions modules/apps/27-interchain-accounts/types/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ import (

crypto "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
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"
"github.com/tendermint/tendermint/crypto/tmhash"
"gopkg.in/yaml.v2"
yaml "gopkg.in/yaml.v2"

connectiontypes "github.com/cosmos/ibc-go/v2/modules/core/03-connection/types"
)

// 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.
// 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(sdkaddress.Derive(moduleAccAddr, []byte(portID)))
}

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,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
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