Skip to content

Commit

Permalink
Problem: interchain-accounts integration is not mature (#672)
Browse files Browse the repository at this point in the history
* Problem: interchain-accounts integration is not mature

Solution:
- revert from 0.9

* fix gravity bridge upgrade handler

* Update CHANGELOG.md

* fix build and lint

* bump gravity-bridge upgrade plan name

* fix gravity upgrade test

* fix gravity integration test
  • Loading branch information
yihuang authored Aug 25, 2022
1 parent 6d14b41 commit 41d451f
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 192 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [cronos#611](https://github.com/crypto-org-chain/cronos/pull/611) Fix mistake on acknowledgement error in ibc middleware.
- [cronos#627](https://github.com/crypto-org-chain/cronos/pull/627) Upgrade gravity bridge module with security enhancements
- [cronos#647](https://github.com/crypto-org-chain/cronos/pull/647) Integrate ibc fee middleware.
- [cronos#672](https://github.com/crypto-org-chain/cronos/pull/672) Revert interchain-accounts integration.

### Bug Fixes

Expand Down
84 changes: 19 additions & 65 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,7 @@ 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"
ica "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts"
icacontroller "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller"
icacontrollerkeeper "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/keeper"
icacontrollertypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
ibcfee "github.com/cosmos/ibc-go/v5/modules/apps/29-fee"
ibcfeekeeper "github.com/cosmos/ibc-go/v5/modules/apps/29-fee/keeper"
ibcfeetypes "github.com/cosmos/ibc-go/v5/modules/apps/29-fee/types"
Expand Down Expand Up @@ -127,9 +123,6 @@ import (
cronoskeeper "github.com/crypto-org-chain/cronos/x/cronos/keeper"
evmhandlers "github.com/crypto-org-chain/cronos/x/cronos/keeper/evmhandlers"
cronostypes "github.com/crypto-org-chain/cronos/x/cronos/types"
icactlmodule "github.com/crypto-org-chain/cronos/x/icactl"
icactlmodulekeeper "github.com/crypto-org-chain/cronos/x/icactl/keeper"
icactlmoduletypes "github.com/crypto-org-chain/cronos/x/icactl/types"

// unnamed import of statik for swagger UI support
_ "github.com/crypto-org-chain/cronos/client/docs/statik"
Expand Down Expand Up @@ -190,7 +183,6 @@ var (
govtypes.ModuleName: {authtypes.Burner},
ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner},
ibcfeetypes.ModuleName: nil,
icatypes.ModuleName: nil,
evmtypes.ModuleName: {authtypes.Minter, authtypes.Burner}, // used for secure addition and subtraction of balance using module account
gravitytypes.ModuleName: {authtypes.Minter, authtypes.Burner},
cronostypes.ModuleName: {authtypes.Minter, authtypes.Burner},
Expand Down Expand Up @@ -231,8 +223,6 @@ func GenModuleBasics(experimental bool) module.BasicManager {
ibc.AppModuleBasic{},
transfer.AppModuleBasic{},
vesting.AppModuleBasic{},
ica.AppModuleBasic{},
icactlmodule.AppModuleBasic{},
ibcfee.AppModuleBasic{},
evm.AppModuleBasic{},
feemarket.AppModuleBasic{},
Expand Down Expand Up @@ -263,31 +253,27 @@ type App struct {
memKeys map[string]*storetypes.MemoryStoreKey

// keepers
AccountKeeper authkeeper.AccountKeeper
BankKeeper bankkeeper.Keeper
CapabilityKeeper *capabilitykeeper.Keeper
StakingKeeper stakingkeeper.Keeper
SlashingKeeper slashingkeeper.Keeper
MintKeeper mintkeeper.Keeper
DistrKeeper distrkeeper.Keeper
GovKeeper govkeeper.Keeper
CrisisKeeper crisiskeeper.Keeper
UpgradeKeeper upgradekeeper.Keeper
ParamsKeeper paramskeeper.Keeper
IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
IBCFeeKeeper ibcfeekeeper.Keeper
ICAControllerKeeper icacontrollerkeeper.Keeper
ICAAuthKeeper icactlmodulekeeper.Keeper
EvidenceKeeper evidencekeeper.Keeper
TransferKeeper ibctransferkeeper.Keeper
FeeGrantKeeper feegrantkeeper.Keeper
AuthzKeeper authzkeeper.Keeper
AccountKeeper authkeeper.AccountKeeper
BankKeeper bankkeeper.Keeper
CapabilityKeeper *capabilitykeeper.Keeper
StakingKeeper stakingkeeper.Keeper
SlashingKeeper slashingkeeper.Keeper
MintKeeper mintkeeper.Keeper
DistrKeeper distrkeeper.Keeper
GovKeeper govkeeper.Keeper
CrisisKeeper crisiskeeper.Keeper
UpgradeKeeper upgradekeeper.Keeper
ParamsKeeper paramskeeper.Keeper
IBCKeeper *ibckeeper.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
IBCFeeKeeper ibcfeekeeper.Keeper
EvidenceKeeper evidencekeeper.Keeper
TransferKeeper ibctransferkeeper.Keeper
FeeGrantKeeper feegrantkeeper.Keeper
AuthzKeeper authzkeeper.Keeper

// make scoped keepers public for test purposes
ScopedIBCKeeper capabilitykeeper.ScopedKeeper
ScopedTransferKeeper capabilitykeeper.ScopedKeeper
ScopedICAControllerKeeper capabilitykeeper.ScopedKeeper
ScopedICAAuthKeeper capabilitykeeper.ScopedKeeper
ScopedIBCKeeper capabilitykeeper.ScopedKeeper
ScopedTransferKeeper capabilitykeeper.ScopedKeeper

// Ethermint keepers
EvmKeeper *evmkeeper.Keeper
Expand Down Expand Up @@ -379,8 +365,6 @@ func New(
// grant capabilities for the ibc and ibc-transfer modules
scopedIBCKeeper := app.CapabilityKeeper.ScopeToModule(ibchost.ModuleName)
scopedTransferKeeper := app.CapabilityKeeper.ScopeToModule(ibctransfertypes.ModuleName)
scopedICAControllerKeeper := app.CapabilityKeeper.ScopeToModule(icacontrollertypes.SubModuleName)
scopedICAAuthKeeper := app.CapabilityKeeper.ScopeToModule(icactlmoduletypes.ModuleName)
// Applications that wish to enforce statically created ScopedKeepers should call `Seal` after creating
// their scoped modules in `NewApp` with `ScopeToModule`
app.CapabilityKeeper.Seal()
Expand Down Expand Up @@ -516,22 +500,6 @@ func New(
transferStack = middleware.NewIBCConversionModule(transferStack, app.CronosKeeper)
transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper)

app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper(
appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName),
app.IBCFeeKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
scopedICAControllerKeeper, app.MsgServiceRouter(),
)
icaModule := ica.NewAppModule(&app.ICAControllerKeeper, nil)

app.ICAAuthKeeper = *icactlmodulekeeper.NewKeeper(appCodec,
app.GetSubspace(icactlmoduletypes.ModuleName), app.ICAControllerKeeper, scopedICAAuthKeeper)
icaCtlModule := icactlmodule.NewAppModule(appCodec, app.ICAAuthKeeper)

var icaControllerStack porttypes.IBCModule
icaControllerStack = icactlmodule.NewIBCModule(app.ICAAuthKeeper)
icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)

app.GovKeeper = govkeeper.NewKeeper(
appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper,
&stakingKeeper, govRouter, app.MsgServiceRouter(), govConfig,
Expand Down Expand Up @@ -573,8 +541,6 @@ func New(
// Create static IBC router, add transfer route, then set and seal it
ibcRouter := porttypes.NewRouter()
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
ibcRouter.AddRoute(icactlmoduletypes.ModuleName, icaControllerStack)
// this line is used by starport scaffolding # ibc/app/router
app.IBCKeeper.SetRouter(ibcRouter)

Expand Down Expand Up @@ -609,8 +575,6 @@ func New(
authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
ibc.NewAppModule(app.IBCKeeper),
transferModule,
icaModule,
icaCtlModule,
feeModule,
evm.NewAppModule(app.EvmKeeper, app.AccountKeeper),
feemarket.NewAppModule(app.FeeMarketKeeper),
Expand All @@ -628,8 +592,6 @@ func New(
minttypes.ModuleName, distrtypes.ModuleName, slashingtypes.ModuleName,
evidencetypes.ModuleName, stakingtypes.ModuleName, ibchost.ModuleName,
ibctransfertypes.ModuleName,
icatypes.ModuleName,
icactlmoduletypes.ModuleName,
ibcfeetypes.ModuleName,
authtypes.ModuleName,
banktypes.ModuleName,
Expand All @@ -648,8 +610,6 @@ func New(
evmtypes.ModuleName, feemarkettypes.ModuleName,
ibchost.ModuleName,
ibctransfertypes.ModuleName,
icatypes.ModuleName,
icactlmoduletypes.ModuleName,
ibcfeetypes.ModuleName,
capabilitytypes.ModuleName,
authtypes.ModuleName,
Expand Down Expand Up @@ -690,8 +650,6 @@ func New(
genutiltypes.ModuleName,
evidencetypes.ModuleName,
ibctransfertypes.ModuleName,
icatypes.ModuleName,
icactlmoduletypes.ModuleName,
ibcfeetypes.ModuleName,
authz.ModuleName,
feegrant.ModuleName,
Expand Down Expand Up @@ -775,8 +733,6 @@ func New(

app.ScopedIBCKeeper = scopedIBCKeeper
app.ScopedTransferKeeper = scopedTransferKeeper
app.ScopedICAControllerKeeper = scopedICAControllerKeeper
app.ScopedICAAuthKeeper = scopedICAAuthKeeper
// this line is used by starport scaffolding # stargate/app/beforeInitReturn

return app
Expand Down Expand Up @@ -974,8 +930,6 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
paramsKeeper.Subspace(crisistypes.ModuleName)
paramsKeeper.Subspace(ibctransfertypes.ModuleName)
paramsKeeper.Subspace(ibchost.ModuleName)
paramsKeeper.Subspace(icacontrollertypes.SubModuleName)
paramsKeeper.Subspace(icactlmoduletypes.ModuleName)
paramsKeeper.Subspace(evmtypes.ModuleName)
paramsKeeper.Subspace(feemarkettypes.ModuleName)
if experimental {
Expand Down
31 changes: 5 additions & 26 deletions app/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
icacontrollertypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
ibcfeetypes "github.com/cosmos/ibc-go/v5/modules/apps/29-fee/types"
gravitytypes "github.com/peggyjv/gravity-bridge/module/v2/x/gravity/types"
)

func (app *App) RegisterUpgradeHandlers(experimental bool) {
Expand All @@ -18,31 +17,10 @@ func (app *App) RegisterUpgradeHandlers(experimental bool) {
return app.mm.RunMigrations(ctx, app.configurator, fromVM)
})

gravityPlanName := "v0.8.0-gravity-alpha1"
gravityPlanName := "v0.8.0-gravity-alpha3"
if experimental {
app.UpgradeKeeper.SetUpgradeHandler(gravityPlanName, func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
updatedVM, err := app.mm.RunMigrations(ctx, app.configurator, fromVM)
if err != nil {
return nil, err
}
// register new params
gravParamStore := app.GetSubspace(gravitytypes.ModuleName)
gravParamStore.Set(ctx, gravitytypes.ParamStoreBridgeActive, true)
gravParamStore.Set(ctx, gravitytypes.ParamStoreBatchCreationPeriod, uint64(10))
gravParamStore.Set(ctx, gravitytypes.ParamStoreBatchMaxElement, uint64(100))
gravParamStore.Set(ctx, gravitytypes.ParamStoreObserveEthereumHeightPeriod, uint64(50))

// set new gravity id
gravParams := app.GravityKeeper.GetParams(ctx)
gravParams.GravityId = "cronos_gravity_pioneer_v2"
app.GravityKeeper.SetParams(ctx, gravParams)

// Estimate time upgrade take place
// 100% is not necessary here because it will be tuned by relayers later on
// it is set to https://goerli.etherscan.io/block/countdown/7460000
app.GravityKeeper.MigrateGravityContract(
ctx, "0x0000000000000000000000000000000000000000", 7460000)
return updatedVM, nil
return app.mm.RunMigrations(ctx, app.configurator, fromVM)
})
}

Expand All @@ -54,15 +32,16 @@ func (app *App) RegisterUpgradeHandlers(experimental bool) {
if !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
if upgradeInfo.Name == planName {
storeUpgrades := storetypes.StoreUpgrades{
Added: []string{icacontrollertypes.StoreKey, ibcfeetypes.StoreKey},
Added: []string{ibcfeetypes.StoreKey},
}

// configure store loader that checks if version == upgradeHeight and applies store upgrades
app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
}
if experimental && upgradeInfo.Name == gravityPlanName {
storeUpgrades := storetypes.StoreUpgrades{
Added: []string{icacontrollertypes.StoreKey},
Added: []string{ibcfeetypes.StoreKey},
Deleted: []string{icacontrollertypes.StoreKey},
}

// configure store loader that checks if version == upgradeHeight and applies store upgrades
Expand Down
5 changes: 3 additions & 2 deletions integration_tests/configs/upgrade-test-package-gravity.nix
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ let
shortRev = builtins.substring 0 7 rev;
};
}).defaultNix;
released = (fetchFlake "crypto-org-chain/cronos" "e935ae247d910e7e11c1d5858766ba49f75298e8").default;
# tag: v0.8.0-gravity-alpha2
released = (fetchFlake "crypto-org-chain/cronos" "57260c7c21cdedffd75480e8cb4e8838ea6a16b5").default;
current = pkgs.callPackage ../../. { };
in
pkgs.linkFarm "upgrade-test-package" [
{ name = "genesis"; path = released; }
{ name = "v0.8.0-gravity-alpha1"; path = current; }
{ name = "v0.8.0-gravity-alpha3"; path = current; }
]
91 changes: 0 additions & 91 deletions integration_tests/test_ibc.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import base64
import json

import pytest

from .ibc_utils import (
Expand Down Expand Up @@ -225,94 +222,6 @@ def check_balance_change():
assert old_dst_balance + dst_amount == new_dst_balance


def test_ica(ibc, tmp_path):
connid = "connection-0"
cli_host = ibc.chainmain.cosmos_cli()
cli_controller = ibc.cronos.cosmos_cli()

print("register ica account")
rsp = cli_controller.ica_register_account(
connid, from_="signer2", gas="400000", fees="100000000basetcro"
)
assert rsp["code"] == 0, rsp["raw_log"]
port_id, channel_id = next(
(
base64.b64decode(evt["attributes"][0]["value"].encode()).decode(),
base64.b64decode(evt["attributes"][1]["value"].encode()).decode(),
)
for evt in rsp["events"]
if evt["type"] == "channel_open_init"
)
print("port-id", port_id, "channel-id", channel_id)

print("wait for ica channel ready")

def check_channel_ready():
channels = cli_controller.ibc_query_channels(connid)["channels"]
try:
state = next(
channel["state"]
for channel in channels
if channel["channel_id"] == channel_id
)
except StopIteration:
return False
return state == "STATE_OPEN"

wait_for_fn("channel ready", check_channel_ready)

print("query ica account")
ica_address = cli_controller.ica_query_account(
connid, cli_controller.address("signer2")
)["interchainAccountAddress"]
print("ica address", ica_address)

# initial balance of interchain account should be zero
assert cli_host.balance(ica_address) == 0

# send some funds to interchain account
rsp = cli_host.transfer("signer2", ica_address, "1cro", gas_prices="1000000basecro")
assert rsp["code"] == 0, rsp["raw_log"]
wait_for_new_blocks(cli_host, 1)

# check if the funds are received in interchain account
assert cli_host.balance(ica_address, denom="basecro") == 100000000

# generate a transaction to send to host chain
generated_tx = tmp_path / "generated_tx.txt"
generated_tx_msg = cli_host.transfer(
ica_address, cli_host.address("signer2"), "0.5cro", generate_only=True
)

print(generated_tx_msg)
generated_tx.write_text(json.dumps(generated_tx_msg))

num_txs = len(cli_host.query_all_txs(ica_address)["txs"])

# submit transaction on host chain on behalf of interchain account
rsp = cli_controller.ica_submit_tx(
connid,
generated_tx,
from_="signer2",
)
assert rsp["code"] == 0, rsp["raw_log"]
packet_seq = next(
int(base64.b64decode(evt["attributes"][4]["value"].encode()))
for evt in rsp["events"]
if evt["type"] == "send_packet"
)
print("packet sequence", packet_seq)

def check_ica_tx():
return len(cli_host.query_all_txs(ica_address)["txs"]) > num_txs

print("wait for ica tx arrive")
wait_for_fn("ica transfer tx", check_ica_tx)

# check if the funds are reduced in interchain account
assert cli_host.balance(ica_address, denom="basecro") == 50000000


def test_cronos_transfer_source_tokens(ibc):
"""
test sending crc20 tokens originated from cronos to crypto-org-chain
Expand Down
4 changes: 0 additions & 4 deletions integration_tests/test_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,5 @@ def test_cosmovisor_upgrade(custom_cronos: Cronos):
wait_for_block(cli, target_height + 2, timeout=480)
wait_for_port(ports.rpc_port(custom_cronos.base_port(0)))

# check ica controller is enabled
assert cli.query_icacontroller_params() == {"controller_enabled": True}
assert cli.query_icactl_params() == {"params": {"minTimeoutDuration": "3600s"}}

# test migrate keystore
cli.migrate_keystore()
Loading

0 comments on commit 41d451f

Please sign in to comment.