Skip to content

Commit

Permalink
Merge pull request #121 from notional-labs/v0.47.x-ibc-ruslan
Browse files Browse the repository at this point in the history
Merge branch 'cosmos:main' into v0.47.x-ibc-ruslan
  • Loading branch information
faddat authored Dec 8, 2022
2 parents 975e533 + 003eee9 commit b56b8e8
Show file tree
Hide file tree
Showing 12 changed files with 135 additions and 35 deletions.
10 changes: 6 additions & 4 deletions .github/ISSUE_TEMPLATE/release-tracker.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ v without deliberation
<!-- List of tests that need be performed with previous
versions of ibc-go to guarantee that no regression is introduced -->

- [ ] Fungible token transfers over a non-incentivised channel works with a counterparty chain running each previous major version.
- [ ] Fungible token transfers over an incentivised channel works with a counterparty chain running each previous major version that supports ICS-29 Fee Middleware.
- [ ] Interchain Accounts over a non-incentivised channel works with a counterparty chain running each previous major version that supports ICS-27 Interchain Accounts over non-incentivised channels.
- [ ] Interchain Accounts over an incentivised channel works with a counterparty chain running each previous major version that supports ICS-27 Interchain Accounts over incentivised channels.
- [ ] [Compatibility tests](https://github.com/cosmos/ibc-go/actions/workflows/e2e-compatibility.yaml) pass for the release branch.
- [ ] [Upgrade tests](https://github.com/cosmos/ibc-go/actions/workflows/e2e-upgrade.yaml) pass.
- [ ] [Interchain Accounts inter-tx tests](https://github.com/cosmos/interchain-accounts-demo/actions/workflows/e2e-compatibility.yaml) pass.

### Other testing

## Migration

Expand Down Expand Up @@ -57,6 +58,7 @@ versions of ibc-go to guarantee that no regression is introduced -->
- [ ] Add new release branch to [`docs/versions`](https://github.com/cosmos/ibc-go/blob/main/docs/versions) file.
- [ ] Add `label` and `key` to `versions` array in [`config.js`](https://github.com/cosmos/ibc-go/blob/main/docs/.vuepress/config.js#L62).
- [ ] After changes to docs site are deployed, check [ibc.cosmos.network](https://ibc.cosmos.network) is updated.
- [ ] Open issue in [SDK tutorials repo](https://github.com/cosmos/sdk-tutorials) to update tutorials to the released version of ibc-go.

____

Expand Down
Binary file added docs/assets/auth-module-decision-tree.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
100 changes: 93 additions & 7 deletions docs/migrations/v5-to-v6.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ This migration facilitates the addition of the ICS27 controller submodule `MsgSe

For more information please refer to [ADR 009](../architecture/adr-009-v6-ics27-msgserver.md).

#### Upgrade proposal
### Upgrade proposal

Please refer to [PR #2383](https://github.com/cosmos/ibc-go/pull/2383) for integrating the ICS27 channel capability migration logic or follow the steps outlined below:

Expand Down Expand Up @@ -87,21 +87,86 @@ app.UpgradeKeeper.SetUpgradeHandler(

#### Controller APIs

In previous releases of ibc-go, chain developers integrating the ICS27 interchain accounts controller functionality were expected to create a custom `Base Application` referred to as an authentication module, see: [Building an authentication module](../apps/interchain-accounts/auth-modules.md).
In previous releases of ibc-go, chain developers integrating the ICS27 interchain accounts controller functionality were expected to create a custom `Base Application` referred to as an authentication module, see the section [Building an authentication module](../apps/interchain-accounts/auth-modules.md) from the documentation.

The `Base Application` was intended to be composed with the ICS27 controller submodule `Keeper` and faciliate many forms of message authentication depending on a chain's particular use case.

The controller submodule exposes two functions:
Prior to ibc-go v6 the controller submodule exposed only these two functions (to which we will refer as the legacy APIs):

- [`RegisterInterchainAccount`](https://github.com/cosmos/ibc-go/blob/v5.0.0/modules/apps/27-interchain-accounts/controller/keeper/account.go#L19)
- [`SendTx`](https://github.com/cosmos/ibc-go/blob/v5.0.0/modules/apps/27-interchain-accounts/controller/keeper/relay.go#L18)

These functions have been deprecated in favour of the new controller submodule `MsgServer` and will be removed in later releases.
Both APIs remain functional and maintain backwards compatibility in `ibc-go/v6`, however consumers of these APIs are now recommended to follow the message passing paradigm outlined in Cosmos SDK [ADR 031](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-031-msg-service.md) and [ADR 033](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-033-protobuf-inter-module-comm.md). This is facilitated by the Cosmos SDK [`MsgServiceRouter`](https://github.com/cosmos/cosmos-sdk/blob/main/baseapp/msg_service_router.go#L17) and chain developers creating custom application logic can now omit the ICS27 controller submodule `Keeper` from their module and instead depend on message routing.
However, these functions have now been deprecated in favour of the new controller submodule `MsgServer` and will be removed in later releases.

Legacy APIs are still required if application developers wish to consume IBC packet callbacks and react upon packet acknowledgements. In future this will be replaced by IBC Actor Callbacks, see [ADR 008](https://github.com/cosmos/ibc-go/pull/1976).
Both APIs remain functional and maintain backwards compatibility in ibc-go v6, however consumers of these APIs are now recommended to follow the message passing paradigm outlined in Cosmos SDK [ADR 031](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-031-msg-service.md) and [ADR 033](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-033-protobuf-inter-module-comm.md). This is facilitated by the Cosmos SDK [`MsgServiceRouter`](https://github.com/cosmos/cosmos-sdk/blob/main/baseapp/msg_service_router.go#L17) and chain developers creating custom application logic can now omit the ICS27 controller submodule `Keeper` from their module and instead depend on message routing.

For more information see the new [ICS27 integration documentation](TODO: add link to new docs).
Depending on the use case, developers of custom authentication modules face one of three scenarios:

![AuthModuleDecisionTree](../assets/auth-module-decision-tree.png)

**My authentication module needs to access IBC packet callbacks**

Application developers that wish to consume IBC packet callbacks and react upon packet acknowledgements **must** continue using the controller submodule's legacy APIs. The authentication modules will not need a `ScopedKeeper` anymore, though, because the channel capability will be claimed by the controller submodule. For example, given an Interchain Accounts authentication module keeper `ICAAuthKeeper`, the authentication module's `ScopedKeeper` (`scopedICAAuthKeeper`) is not needed anymore and can be removed for the argument list of the keeper constructor function, as shown here:

```diff
app.ICAAuthKeeper = icaauthkeeper.NewKeeper(
appCodec,
keys[icaauthtypes.StoreKey],
app.ICAControllerKeeper,
- scopedICAAuthKeeper,
)
```

Please note that the authentication module's `ScopedKeeper` name is still needed as part of the channel capability migration described in section [Upgrade proposal](#upgrade-proposal) above. Therefore the authentication module's `ScopedKeeper` cannot be completely removed from the chain code until the migration has run.

In the future, the use of the legacy APIs for accessing packet callbacks will be replaced by IBC Actor Callbacks (see [ADR 008](https://github.com/cosmos/ibc-go/pull/1976) for more details) and it will also be possible to access them with the `MsgServiceRouter`.

**My authentication module does not need access to IBC packet callbacks**

The authentication module can migrate from using the legacy APIs and it can be composed instead with the `MsgServiceRouter`, so that the authentication module is able to pass messages to the controller submodule's `MsgServer` to register interchain accounts and send packets to the interchain account. For example, given an Interchain Accounts authentication module keeper `ICAAuthKeeper`, the ICS27 controller submodule keeper (`ICAControllerKeeper`) and authentication module scoped keeper (`scopedICAAuthKeeper`) are not needed anymore and can be replaced with the `MsgServiceRouter`, as shown here:

```diff
app.ICAAuthKeeper = icaauthkeeper.NewKeeper(
appCodec,
keys[icaauthtypes.StoreKey],
- app.ICAControllerKeeper,
- scopedICAAuthKeeper,
+ app.MsgServiceRouter(),
)
```

In your authentication module you can route messages to the controller submodule's `MsgServer` instead of using the legacy APIs. For example, for registering an interchain account:

```diff
- if err := keeper.icaControllerKeeper.RegisterInterchainAccount(
- ctx,
- connectionID,
- owner.String(),
- version,
- ); err != nil {
- return err
- }
+ msg := controllertypes.NewMsgRegisterInterchainAccount(
+ connectionID,
+ owner.String(),
+ version,
+ )
+ handler := keeper.msgRouter.Handler(msg)
+ res, err := handler(ctx, msg)
+ if err != nil {
+ return err
+ }
```

where `controllertypes` is an import alias for `"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"`.

In addition, in this use case the authentication module does not need to implement the `IBCModule` interface anymore.

**I do not need a custom authentication module anymore**

If your authentication module does not have any extra functionality compared to the default authentication module added in ibc-go v6 (the `MsgServer`), or if you can use a generic authentication module, such as the `x/auth`, `x/gov` or `x/group` modules from the Cosmos SDK (v0.46 and later), then you can remove your authentication module completely and use instead the gRPC endpoints of the `MsgServer` or the CLI added in ibc-go v6.

Please remember that the authentication module's `ScopedKeeper` name is still needed as part of the channel capability migration described in section [Upgrade proposal](#upgrade-proposal) above.

#### Host params

Expand Down Expand Up @@ -196,6 +261,27 @@ Callers no longer need to pass in a pre-constructed packet.
The destination port/channel identifiers and the packet sequence will be determined by core IBC.
`SendPacket` will return the packet sequence.

### IBC testing package

The `SendPacket` API has been simplified:

```diff
// SendPacket is called by a module in order to send an IBC packet on a channel
func (k Keeper) SendPacket(
ctx sdk.Context,
channelCap *capabilitytypes.Capability,
- packet exported.PacketI,
-) error {
+ sourcePort string,
+ sourceChannel string,
+ timeoutHeight clienttypes.Height,
+ timeoutTimestamp uint64,
+ data []byte,
+) (uint64, error) {
```

Callers no longer need to pass in a pre-constructed packet. `SendPacket` will return the packet sequence.

## Relayers

- No relevant changes were made in this release.
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (suite *InterchainAccountsTestSuite) TestInitModule() {
Time: suite.coordinator.CurrentTime.UTC(),
}

ctx := app.GetBaseApp().NewContext(true, header)
ctx := app.BaseApp.NewContext(true, header)

// ensure params are not set
suite.Require().Panics(func() {
Expand Down Expand Up @@ -114,7 +114,7 @@ func (suite *InterchainAccountsTestSuite) TestInitModule() {
Time: suite.coordinator.CurrentTime.UTC(),
}

ctx := app.GetBaseApp().NewContext(true, header)
ctx := app.BaseApp.NewContext(true, header)

tc.malleate()

Expand Down
14 changes: 0 additions & 14 deletions testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,21 +706,7 @@ func (app *SimApp) LoadHeight(height int64) error {
return app.LoadVersion(height)
}

// ModuleAccountAddrs returns all the app's module account addresses.
func (app *SimApp) ModuleAccountAddrs() map[string]bool {
modAccAddrs := make(map[string]bool)
for acc := range maccPerms {
// do not add the following modules to blocked addresses
// this is only used for testing
if acc == ibcmock.ModuleName {
continue
}

modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true
}

return modAccAddrs
}

// GetModuleManager returns the app module manager
// NOTE: used for testing purposes
Expand Down
8 changes: 8 additions & 0 deletions testing/simapp/app_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ import (
upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client"
upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
ibcfeetypes "github.com/cosmos/ibc-go/v6/modules/apps/29-fee/types"
ibcmock "github.com/cosmos/ibc-go/v6/testing/mock"
)

// IBC application testing ports
const (
MockFeePort string = ibcmock.ModuleName + ibcfeetypes.ModuleName

)

var (
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/sim_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func BenchmarkFullAppSimulation(b *testing.B) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down
6 changes: 3 additions & 3 deletions testing/simapp/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestAppImportExport(t *testing.T) {

fmt.Printf("exporting genesis...\n")

exported, err := app.ExportAppStateAndValidators(false, []string{})
exported, err := app.ExportAppStateAndValidators(false, []string{}, []string{})
require.NoError(t, err)

fmt.Printf("importing genesis...\n")
Expand Down Expand Up @@ -252,7 +252,7 @@ func TestAppSimulationAfterImport(t *testing.T) {

fmt.Printf("exporting genesis...\n")

exported, err := app.ExportAppStateAndValidators(true, []string{})
exported, err := app.ExportAppStateAndValidators(true, []string{}, []string{})
require.NoError(t, err)

fmt.Printf("importing genesis...\n")
Expand All @@ -275,7 +275,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
_, _, err = simulation.SimulateFromSeed(
t,
os.Stdout,
newApp.GetBaseApp(),
newApp.BaseApp,
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(newApp, newApp.AppCodec(), config),
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,5 +319,5 @@ func (a appCreator) appExport(
simApp = simapp.NewSimApp(logger, db, traceStore, true, map[int64]bool{}, homePath, uint(1), a.encCfg, appOpts)
}

return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs)
return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs, modulesToExport)
}
20 changes: 19 additions & 1 deletion testing/simapp/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
dbm "github.com/tendermint/tm-db"

"github.com/cosmos/ibc-go/v6/testing/mock"
ibcmock "github.com/cosmos/ibc-go/v6/testing/mock"
)

// DefaultConsensusParams defines the default Tendermint consensus params used in
Expand All @@ -42,13 +43,30 @@ var DefaultConsensusParams = &tmproto.ConsensusParams{
},
}

// ModuleAccountAddrs returns all the app's module account addresses.
func (app *SimApp) ModuleAccountAddrs() map[string]bool {
modAccAddrs := make(map[string]bool)
for acc := range maccPerms {
// do not add the following modules to blocked addresses
// this is only used for testing
if acc == ibcmock.ModuleName {
continue
}

modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true
}

return modAccAddrs
}

func setup(withGenesis bool, invCheckPeriod uint) (*SimApp, GenesisState) {
db := dbm.NewMemDB()
appOptions := make(simtestutil.AppOptionsMap, 0)
appOptions[flags.FlagHome] = DefaultNodeHome
appOptions[server.FlagInvCheckPeriod] = invCheckPeriod

app := NewSimApp(log.NewNopLogger(), db, nil, true, appOptions)
app := NewSimApp(log.NewNopLogger(), db, nil, true, simtestutil.EmptyAppOptions{})

if withGenesis {
return app, NewDefaultGenesisState(app.AppCodec())
}
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type App interface {

// Exports the state of the application for a genesis file.
ExportAppStateAndValidators(
forZeroHeight bool, jailAllowedAddrs []string,
forZeroHeight bool, jailAllowedAddrs []string, modulesToExport []string,
) (types.ExportedApp, error)

// All the registered module account addreses.
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func CheckExportSimulation(
) error {
if config.ExportStatePath != "" {
fmt.Println("exporting app state...")
exported, err := app.ExportAppStateAndValidators(false, nil)
exported, err := app.ExportAppStateAndValidators(false, nil, nil)
if err != nil {
return err
}
Expand Down

0 comments on commit b56b8e8

Please sign in to comment.