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

refactor: use header info for interchain accounts address generation #7713

Merged
merged 2 commits into from
Dec 18, 2024
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
10 changes: 5 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,14 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Testing

* [\#7430](https://github.com/cosmos/ibc-go/pull/7430) Update the block proposer in test chains for each block.

### Dependencies

* [\#7540](https://github.com/cosmos/ibc-go/pull/7540) Bump CometBFT to v0.38.15.
* [\#7261](https://github.com/cosmos/ibc-go/pull/7261) Bump CometBFT to v1.0.0.
* [\#7261](https://github.com/cosmos/ibc-go/pull/7261) Bump Cosmos SDK to v0.52.0.

### API Breaking

* (apps/27-interchain-accounts) [\#7713](https://github.com/cosmos/ibc-go/pull/7713) Update interchain accounts `GenerateAddress` func to now accept `header.Info` in favour of `sdk.Context`. This function now uses `AppHash` and `Hash` (merkle root of block) instead of `AppHash` and `DataHash` as pre-image data for address generation.
* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7213) Remove capabilities from `SendPacket`.
* (core, apps) [\#7213](https://github.com/cosmos/ibc-go/pull/7225) Remove capabilities from `WriteAcknowledgement`.
* (core, apps) [\#7232](https://github.com/cosmos/ibc-go/pull/7232) Remove capabilities from channel handshake methods. TODO list all changes
Expand All @@ -63,6 +61,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (testing)[\#7430](https://github.com/cosmos/ibc-go/pull/7430) Update the block proposer in test chains for each block.

### Features

### Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
},
}

interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, TestPortID)
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext().HeaderInfo(), ibctesting.FirstConnectionID, TestPortID)
genesisState := genesistypes.ControllerGenesisState{
ActiveChannels: []genesistypes.ActiveChannel{
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() {
},
{
"account address generation is block dependent", func() {
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext().HeaderInfo(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
interchainAcc := icatypes.NewInterchainAccount(authtypes.NewBaseAccountWithAddress(interchainAccAddr), path.EndpointA.ChannelConfig.PortID)
suite.chainB.GetSimApp().AuthKeeper.NewAccount(suite.chainB.GetContext(), interchainAcc)
suite.chainB.GetSimApp().AuthKeeper.SetAccount(suite.chainB.GetContext(), interchainAcc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// and block dependent information. An error is returned if an account already exists for the generated account.
// An interchain account type is set in the account keeper and the interchain account address mapping is updated.
func (k Keeper) createInterchainAccount(ctx context.Context, connectionID, controllerPortID string) (sdk.AccAddress, error) {
accAddress := icatypes.GenerateAddress(ctx, connectionID, controllerPortID)
accAddress := icatypes.GenerateAddress(k.HeaderService.HeaderInfo(ctx), connectionID, controllerPortID)

if acc := k.authKeeper.GetAccount(ctx, accAddress); acc != nil {
return nil, errorsmod.Wrapf(icatypes.ErrAccountAlreadyExist, "existing account for newly generated interchain account address %s", accAddress)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func (suite *KeeperTestSuite) TestInitGenesis() {
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, TestPortID)
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext().HeaderInfo(), ibctesting.FirstConnectionID, TestPortID)
genesisState := genesistypes.HostGenesisState{
ActiveChannels: []genesistypes.ActiveChannel{
{
Expand Down Expand Up @@ -65,7 +65,7 @@ func (suite *KeeperTestSuite) TestGenesisParams() {

suite.Run(tc.name, func() {
suite.SetupTest() // reset
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, TestPortID)
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext().HeaderInfo(), ibctesting.FirstConnectionID, TestPortID)
genesisState := genesistypes.HostGenesisState{
ActiveChannels: []genesistypes.ActiveChannel{
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
{
"account already exists",
func() {
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
interchainAccAddr := icatypes.GenerateAddress(suite.chainB.GetContext().HeaderInfo(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID)
interchainAcc := icatypes.NewInterchainAccount(authtypes.NewBaseAccountWithAddress(interchainAccAddr), path.EndpointA.ChannelConfig.PortID)
suite.chainB.GetSimApp().AuthKeeper.NewAccount(suite.chainB.GetContext(), interchainAcc)
suite.chainB.GetSimApp().AuthKeeper.SetAccount(suite.chainB.GetContext(), interchainAcc)
Expand Down
12 changes: 5 additions & 7 deletions modules/apps/27-interchain-accounts/types/account.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package types

import (
"context"
"encoding/json"
"regexp"
"strings"

yaml "gopkg.in/yaml.v2"

"cosmossdk.io/core/header"
errorsmod "cosmossdk.io/errors"

crypto "github.com/cosmos/cosmos-sdk/crypto/types"
Expand Down Expand Up @@ -43,15 +43,13 @@ type interchainAccountPretty struct {
}

// GenerateAddress returns an sdk.AccAddress derived using a host module account address, host connection ID, the controller portID,
// the current block app hash, and the current block data hash. The sdk.AccAddress returned is a sub-address of the host module account.
func GenerateAddress(ctx context.Context, connectionID, portID string) sdk.AccAddress {
// the current block app hash, and the current block hash (merkle root of block). The sdk.AccAddress returned is a sub-address of the host module account.
func GenerateAddress(headerInfo header.Info, connectionID, portID string) sdk.AccAddress {
hostModuleAcc := sdkaddress.Module(ModuleName, []byte(hostAccountsKey))
sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
header := sdkCtx.BlockHeader()

buf := []byte(connectionID + portID)
buf = append(buf, header.AppHash...)
buf = append(buf, header.DataHash...)
buf = append(buf, headerInfo.AppHash...)
buf = append(buf, headerInfo.Hash...)
Copy link
Member

Choose a reason for hiding this comment

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

I guess we no longer get access to datahash directly? Is there a reason it isn't in header info?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a question for the sdk team I think.


return sdkaddress.Derive(hostModuleAcc, buf)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestTypesTestSuite(t *testing.T) {
}

func (suite *TypesTestSuite) TestGenerateAddress() {
addr := types.GenerateAddress(suite.chainA.GetContext(), "test-connection-id", "test-port-id")
addr := types.GenerateAddress(suite.chainA.GetContext().HeaderInfo(), "test-connection-id", "test-port-id")
accAddr, err := sdk.AccAddressFromBech32(addr.String())

suite.Require().NoError(err, "TestGenerateAddress failed")
Expand Down
6 changes: 6 additions & 0 deletions testing/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,13 @@ func NewTestChain(t *testing.T, coord *Coordinator, chainID string) *TestChain {
func (chain *TestChain) GetContext() sdk.Context {
ctx := chain.App.GetBaseApp().NewUncachedContext(false, chain.ProposedHeader)

cmtHeader, err := cmttypes.HeaderFromProto(&chain.ProposedHeader)
require.NoError(chain.TB, err)

// since:cosmos-sdk/v0.52 when fetching time from context, it now returns from HeaderInfo
headerInfo := header.Info{
AppHash: chain.ProposedHeader.AppHash,
Hash: cmtHeader.Hash(),
Time: chain.ProposedHeader.Time,
ChainID: chain.ProposedHeader.ChainID,
}
Expand Down Expand Up @@ -337,6 +342,7 @@ func (chain *TestChain) commitBlock(res *abci.FinalizeBlockResponse) {

// increment the current header
chain.ProposedHeader = cmtproto.Header{
Version: cmtprotoversion.Consensus{Block: cmtversion.BlockProtocol, App: 1},
ChainID: chain.ChainID,
Height: chain.App.LastBlockHeight() + 1,
AppHash: chain.App.LastCommitID().Hash,
Expand Down
Loading