From 99747c3cec6f9b68092cec443ff527facc465b4d Mon Sep 17 00:00:00 2001 From: Matija Salopek Date: Fri, 5 May 2023 09:34:15 +0200 Subject: [PATCH] fix: refactor difftest setup procedure --- Makefile | 4 ++-- app/consumer/app.go | 8 +------- app/provider/app.go | 5 +---- legacy_ibc_testing/testing/app.go | 21 --------------------- legacy_ibc_testing/testing/chain.go | 10 ---------- tests/difference/core/driver/core_test.go | 2 +- tests/difference/core/driver/setup.go | 16 ++++++++++++++++ tests/integration/setup.go | 5 ----- testutil/ibc_testing/generic_setup.go | 13 ------------- testutil/keeper/mocks.go | 3 --- x/ccv/consumer/keeper/genesis.go | 9 --------- x/ccv/consumer/keeper/validators.go | 3 --- x/ccv/consumer/module.go | 1 - 13 files changed, 21 insertions(+), 79 deletions(-) diff --git a/Makefile b/Makefile index 8a1254e087..c054456ff1 100644 --- a/Makefile +++ b/Makefile @@ -5,8 +5,8 @@ install: go.sum export CGO_CPPFLAGS="-D_FORTIFY_SOURCE=2" export CGO_LDFLAGS="-Wl,-z,relro,-z,now -fstack-protector" go install $(BUILD_FLAGS) ./cmd/interchain-security-pd -# go install $(BUILD_FLAGS) ./cmd/interchain-security-cd -# go install $(BUILD_FLAGS) ./cmd/interchain-security-cdd + go install $(BUILD_FLAGS) ./cmd/interchain-security-cd + go install $(BUILD_FLAGS) ./cmd/interchain-security-cdd # run all tests: unit, integration, diff, and E2E test: diff --git a/app/consumer/app.go b/app/consumer/app.go index e94453841b..146fc01870 100644 --- a/app/consumer/app.go +++ b/app/consumer/app.go @@ -611,17 +611,11 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { var genesisState GenesisState if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil { - // fmt.Println("CONSUGEN:// ERRD OND tmjsonUnmarshal genesis", err) panic(err) } app.UpgradeKeeper.SetModuleVersionMap(ctx, app.MM.GetVersionMap()) - // // fmt.Println(string(req.AppStateBytes)) - // fmt.Println("CONSUGEN:// EXEC INIT GENESIS ###") - // fmt.Println("## CALLING INIT GENESIS from app.go ###") - val := app.MM.InitGenesis(ctx, app.appCodec, genesisState) - // fmt.Println("CONSUGEN:// RAN GENESIS WITH NO ISSUES ###") - return val + return app.MM.InitGenesis(ctx, app.appCodec, genesisState) } // LoadHeight loads a particular height diff --git a/app/provider/app.go b/app/provider/app.go index 849f710842..faaa1dd7af 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -710,10 +710,7 @@ func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.Res app.UpgradeKeeper.SetModuleVersionMap(ctx, app.MM.GetVersionMap()) - vals := app.MM.InitGenesis(ctx, app.appCodec, genesisState) - // fmt.Println("PPPPL//:: REQ ABCI VALSET", req.Validators) - // fmt.Println("PPPPL//:: PROVIDER VALSET UPDATES ###", len(vals.Validators), "\n\n", vals.Validators) - return vals + return app.MM.InitGenesis(ctx, app.appCodec, genesisState) } // LoadHeight loads a particular height diff --git a/legacy_ibc_testing/testing/app.go b/legacy_ibc_testing/testing/app.go index 26f8b12eeb..792316ca1e 100644 --- a/legacy_ibc_testing/testing/app.go +++ b/legacy_ibc_testing/testing/app.go @@ -66,13 +66,6 @@ type TestingApp interface { func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.ValidatorSet, genAccs []authtypes.GenesisAccount, chainID string, powerReduction math.Int, balances ...banktypes.Balance) TestingApp { t.Helper() app, genesisState := appIniter() - // for k, _ := range genesisState { - // fmt.Println(k, "genesis") - // if k == "ccvconsumer" { - // fmt.Println(string(genesisState[k])) - // } - // } - // fmt.Println("AA::// EXECUTED APP INITER ###") baseapp.SetChainID(chainID)(app.GetBaseApp()) // set genesis accounts @@ -113,7 +106,6 @@ func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.V PubKey: pub.PubKey, }) } - // fmt.Println("DONE VALS", len(validators), len(delegations)) // set validators and delegations var ( @@ -128,14 +120,11 @@ func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.V bondDenom = sdk.DefaultBondDenom } - // fmt.Println("DONE STAKING") - // add bonded amount to bonded pool module account balances = append(balances, banktypes.Balance{ Address: authtypes.NewModuleAddress(stakingtypes.BondedPoolName).String(), Coins: sdk.Coins{sdk.NewCoin(bondDenom, bondAmt.Mul(sdk.NewInt(int64(len(valSet.Validators)))))}, }) - // fmt.Println("DONE balances", len(validators), len(delegations)) // set validators and delegations stakingGenesis = *stakingtypes.NewGenesisState(stakingGenesis.Params, validators, delegations) @@ -148,19 +137,13 @@ func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.V if genesisState[consumertypes.ModuleName] != nil { app.AppCodec().MustUnmarshalJSON(genesisState[consumertypes.ModuleName], &consumerGenesis) consumerGenesis.InitialValSet = initValPowers - // consumerGenesis.NewChain = true consumerGenesis.Params.Enabled = true - // fmt.Println("##", len(consumerGenesis.InitialValSet)) genesisState[consumertypes.ModuleName] = app.AppCodec().MustMarshalJSON(&consumerGenesis) } stateBytes, err := json.MarshalIndent(genesisState, "", " ") - // fmt.Println(string(stateBytes)) require.NoError(t, err) - // fmt.Println("DONE STATE BYTES") - // fmt.Println("TRY INIT") - // // fmt.Println("## STATE ##\n\n", string(stateBytes)) // init chain will set the validator set and initialize the genesis accounts app.InitChain( abci.RequestInitChain{ @@ -170,11 +153,9 @@ func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.V AppStateBytes: stateBytes, }, ) - // fmt.Println("DONE INIT") // commit genesis changes app.Commit() - // fmt.Println("DONE COMMIT") app.BeginBlock( abci.RequestBeginBlock{ @@ -188,7 +169,5 @@ func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.V }, ) - // fmt.Printf("$$$$$$$$$$$$$$$$$$$ done on %v\n\n", chainID) - return app } diff --git a/legacy_ibc_testing/testing/chain.go b/legacy_ibc_testing/testing/chain.go index 88dc3343dc..48b6c21bd4 100644 --- a/legacy_ibc_testing/testing/chain.go +++ b/legacy_ibc_testing/testing/chain.go @@ -111,9 +111,6 @@ func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppInite genBals := []banktypes.Balance{} senderAccs := []SenderAccount{} - // fmt.Println("GGG://: INIT NewTestChainWithValSet") - // fmt.Println("NewTestChainWithValSet called", chainID) - // generate genesis accounts for i := 0; i < MaxAccounts; i++ { senderPrivKey := secp256k1.GenPrivKey() @@ -136,11 +133,8 @@ func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppInite senderAccs = append(senderAccs, senderAcc) } - // fmt.Println("GGG://: GEN ACCS", len(genAccs), len(genBals), len(senderAccs)) - // fmt.Println("GGG://: ABOUT TO RUN SetupWithGenesisValSet") app := SetupWithGenesisValSet(t, appIniter, valSet, genAccs, chainID, sdk.DefaultPowerReduction, genBals...) - // fmt.Println("GGG://: DONE SetupWithGenesisValSet") // create current header and call begin block header := tmproto.Header{ @@ -148,10 +142,8 @@ func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppInite Height: 1, Time: coord.CurrentTime.UTC(), } - // fmt.Println("GGG://: WROTE HEADER") txConfig := app.GetTxConfig() - // fmt.Println("GGG://: GOT TX CONFIG") // create an account to send transactions from chain := &TestChain{ @@ -171,10 +163,8 @@ func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppInite SenderAccount: senderAccs[0].SenderAccount, SenderAccounts: senderAccs, } - // fmt.Println("GGG://: HAVE TEST CHAIN") coord.CommitBlock(chain) - // fmt.Println("GGG://: COMMITED BLOCK") return chain } diff --git a/tests/difference/core/driver/core_test.go b/tests/difference/core/driver/core_test.go index 6a215b2e49..2f1b0797d4 100644 --- a/tests/difference/core/driver/core_test.go +++ b/tests/difference/core/driver/core_test.go @@ -185,7 +185,7 @@ func (s *CoreSuite) consumerSlash(val sdk.ConsAddress, h int64, isDowntime bool) } ctx := s.ctx(C) before := len(ctx.EventManager().Events()) - s.consumerKeeper().Slash(ctx, val, h, 0, sdk.Dec{}, kind) + s.consumerKeeper().SlashWithInfractionReason(ctx, val, h, 0, sdk.Dec{}, kind) // consumer module emits packets on slash, so these must be collected. evts := ctx.EventManager().ABCIEvents() for _, e := range evts[before:] { diff --git a/tests/difference/core/driver/setup.go b/tests/difference/core/driver/setup.go index 15bddba444..438d72c821 100644 --- a/tests/difference/core/driver/setup.go +++ b/tests/difference/core/driver/setup.go @@ -157,6 +157,7 @@ func (b *Builder) getAppBytesAndSenders( // Sum bonded is needed for BondedPool account sumBonded := sdk.NewInt(0) + initValPowers := []abci.ValidatorUpdate{} for i, val := range validators.Validators { status := b.initState.ValStates.Status[i] @@ -196,10 +197,18 @@ func (b *Builder) getAppBytesAndSenders( delegations = append(delegations, stakingtypes.NewDelegation(accounts[0].GetAddress(), val.Address.Bytes(), delShares)) // Remaining delegation is from extra account delegations = append(delegations, stakingtypes.NewDelegation(accounts[1].GetAddress(), val.Address.Bytes(), sumShares.Sub(delShares))) + + // add initial validator powers so consumer InitGenesis runs correctly + pub, _ := val.ToProto() + initValPowers = append(initValPowers, abci.ValidatorUpdate{ + Power: val.VotingPower, + PubKey: pub.PubKey, + }) } bondDenom := sdk.DefaultBondDenom genesisStaking := stakingtypes.GenesisState{} + genesisConsumer := consumertypes.GenesisState{} if genesis[stakingtypes.ModuleName] != nil { // If staking module genesis already exists @@ -207,6 +216,13 @@ func (b *Builder) getAppBytesAndSenders( bondDenom = genesisStaking.Params.BondDenom } + if genesis[consumertypes.ModuleName] != nil { + app.AppCodec().MustUnmarshalJSON(genesis[consumertypes.ModuleName], &genesisConsumer) + genesisConsumer.InitialValSet = initValPowers + genesisConsumer.Params.Enabled = true + genesis[consumertypes.ModuleName] = app.AppCodec().MustMarshalJSON(&genesisConsumer) + } + // Set model parameters genesisStaking.Params.MaxEntries = uint32(b.initState.MaxEntries) genesisStaking.Params.MaxValidators = uint32(b.initState.MaxValidators) diff --git a/tests/integration/setup.go b/tests/integration/setup.go index e4b912acd6..70342db186 100644 --- a/tests/integration/setup.go +++ b/tests/integration/setup.go @@ -121,10 +121,7 @@ func (suite *CCVTestSuite) SetupTest() { suite.consumerBundles = make(map[string]*icstestingutils.ConsumerBundle) for i := 0; i < numConsumers; i++ { bundle := suite.setupConsumerCallback(&suite.Suite, suite.coordinator, i) - // fmt.Println("setupConsumerCallback done for bundle") suite.consumerBundles[bundle.Chain.ChainID] = bundle - // TODO: remove break; this will make the tests initialize a single consumer to ease debugging - // break } // initialize each consumer chain with it's corresponding genesis state @@ -157,7 +154,6 @@ func initConsumerChain( chainID string, genesisState *consumertypes.GenesisState, ) { - // fmt.Println("-----> initConsumerChain <-----") providerKeeper := s.providerApp.GetProviderKeeper() bundle := s.consumerBundles[chainID] @@ -170,7 +166,6 @@ func initConsumerChain( // confirm client and cons state for consumer were set correctly in InitGenesis; // NOTE: on restart, both ProviderClientState and ProviderConsensusState are nil - // fmt.Println("?? new chain:", chainID, genesisState.NewChain, len(genesisState.InitialValSet)) if genesisState.NewChain { consumerEndpointClientState, consumerEndpointConsState := s.GetConsumerEndpointClientAndConsState(*bundle) diff --git a/testutil/ibc_testing/generic_setup.go b/testutil/ibc_testing/generic_setup.go index e363f4e284..66f2098f6f 100644 --- a/testutil/ibc_testing/generic_setup.go +++ b/testutil/ibc_testing/generic_setup.go @@ -90,19 +90,15 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( index int, appIniter ibctesting.AppIniter, ) *ConsumerBundle { - // fmt.Println("ADD_CONSU:// ### RUNNING ADD CONSUMER ###") // consumer chain ID chainID := ibctesting.GetChainID(index + 2) - // fmt.Println("AddConsumer WAS CALLED", chainID) // create client to the consumer on the provider chain providerChain := coordinator.Chains[provChainID] providerApp := providerChain.App.(Tp) providerKeeper := providerApp.GetProviderKeeper() - // fmt.Println("ADD_CONSU:// ### GOT PROVI ###") prop := testkeeper.GetTestConsumerAdditionProp() - // fmt.Println("ADD_CONSU:// ### MADE PROP ###") prop.ChainId = chainID // NOTE: the initial height passed to CreateConsumerClient // must be the height on the consumer when InitGenesis is called @@ -112,9 +108,7 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( prop, ) s.Require().NoError(err) - // fmt.Println("ADD_CONSU:// ### ADDED CLIENTS ###") - // fmt.Println("ADD_CONSU:// ### TRY GENESIS ###") // commit the state on the provider chain coordinator.CommitBlock(providerChain) @@ -124,9 +118,7 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( chainID, ) s.Require().True(found, "consumer genesis not found") - // fmt.Println("ADD_CONSU:// ### DONE GENESIS ###") - // fmt.Println("ADD_CONSU:// ### TRY LOOP VALIDATORS ###") // use InitialValSet as the valset on the consumer var valz []*tmtypes.Validator for _, update := range consumerGenesisState.InitialValSet { @@ -140,17 +132,12 @@ func AddConsumer[Tp testutil.ProviderApp, Tc testutil.ConsumerApp]( ProposerPriority: 0, }) } - // fmt.Println("AddConsumer initial valset", len(valz)) - // fmt.Println("ADD_CONSU:// ### DONE LOOP VALIDATORS ###", len(valz)) - // fmt.Println("ADD_CONSU:// ### ABOUT TO RUN NewTestChainWithValSet ###") // create and instantiate consumer chain - // fmt.Println("createConsu", len(valz)) testChain := ibctesting.NewTestChainWithValSet(s.T(), coordinator, appIniter, chainID, tmtypes.NewValidatorSet(valz), providerChain.Signers) coordinator.Chains[chainID] = testChain - // fmt.Println("ADD_CONSU:// ### NewTestChainWithValSet DONE ###") consumerToReturn, ok := testChain.App.(Tc) if !ok { panic(fmt.Sprintf("consumer app type returned from app initer does not match app type passed in as type param: %T, %T", diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index 4cd8fa058d..7bc72cd92a 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -5,7 +5,6 @@ package keeper import ( - "fmt" context "context" reflect "reflect" time "time" @@ -252,7 +251,6 @@ func (m *MockStakingKeeper) Slash(arg0 types0.Context, arg1 types0.ConsAddress, // Slash indicates an expected call of Slash. func (mr *MockStakingKeeperMockRecorder) Slash(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - fmt.Println("Slash") return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Slash", reflect.TypeOf((*MockStakingKeeper)(nil).Slash), arg0, arg1, arg2, arg3, arg4) } @@ -267,7 +265,6 @@ func (m *MockStakingKeeper) SlashWithInfractionReason(arg0 types0.Context, arg1 // SlashWithInfractionReason indicates an expected call of Slash. func (mr *MockStakingKeeperMockRecorder) SlashWithInfractionReason(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - fmt.Println("SlashWithInfractionReason") return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SlashWithInfractionReason", reflect.TypeOf((*MockStakingKeeper)(nil).Slash), arg0, arg1, arg2, arg3, arg4, arg5) } diff --git a/x/ccv/consumer/keeper/genesis.go b/x/ccv/consumer/keeper/genesis.go index 8a956aa649..d27c5e6e17 100644 --- a/x/ccv/consumer/keeper/genesis.go +++ b/x/ccv/consumer/keeper/genesis.go @@ -17,14 +17,12 @@ import ( // 2. A consumer chain restarts after a client to the provider was created, but the CCV channel handshake is still in progress // 3. A consumer chain restarts after the CCV channel handshake was completed. func (k Keeper) InitGenesis(ctx sdk.Context, state *consumertypes.GenesisState) []abci.ValidatorUpdate { - // fmt.Println("### MAIN CONSUMER GENESIS ###", state.PreCCV, state.NewChain, len(state.InitialValSet)) // PreCCV is true during the process of a standalone to consumer changeover. // At the PreCCV point in the process, the standalone chain has just been upgraded to include // the consumer ccv module, but the standalone staking keeper is still managing the validator set. // Once the provider validator set starts validating blocks, the consumer CCV module // will take over proof of stake capabilities, but the standalone staking keeper will // stick around for slashing/jailing purposes. - // fmt.Println("Consumer.InitGenesis called", state.NewChain, len(state.InitialValSet)) if state.PreCCV { k.SetPreCCVTrue(ctx) k.MarkAsPrevStandaloneChain(ctx) @@ -36,7 +34,6 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *consumertypes.GenesisState) // TODO: Remove enabled flag and find a better way to setup integration tests // See: https://github.com/cosmos/interchain-security/issues/339 if !state.Params.Enabled { - // fmt.Println("### SET PARAMS NOT ENABLED") return nil } @@ -56,14 +53,10 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *consumertypes.GenesisState) // initialValSet is checked in NewChain case by ValidateGenesis // start a new chain - // fmt.Println("!!! new chain on first call:", state.NewChain, len(state.InitialValSet)) if state.NewChain { - // fmt.Println("## Before Create Client ") // create the provider client in InitGenesis for new consumer chain. CCV Handshake must be established with this client id. clientID, err := k.clientKeeper.CreateClient(ctx, state.ProviderClientState, state.ProviderConsensusState) - // fmt.Println("## After Create Client ") if err != nil { - // fmt.Println("## PANIC IN InitGenesis ") // If the client creation fails, the chain MUST NOT start panic(err) } @@ -110,13 +103,11 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *consumertypes.GenesisState) } if state.PreCCV { - // fmt.Println("PRE CCV") return []abci.ValidatorUpdate{} } // populate cross chain validators states with initial valset k.ApplyCCValidatorChanges(ctx, state.InitialValSet) - // fmt.Println("## Consumer.InitGenesis INIT VALSET ###", len(state.InitialValSet)) return state.InitialValSet } diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 4caf59dea6..b1b16e4252 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -1,7 +1,6 @@ package keeper import ( - "fmt" "time" "cosmossdk.io/math" @@ -115,7 +114,6 @@ func (k Keeper) Slash(ctx sdk.Context, addr sdk.ConsAddress, infractionHeight, p // All queued slashing requests will be cleared in EndBlock // Called by Slashing keeper in SlashWithInfractionReason func (k Keeper) SlashWithInfractionReason(ctx sdk.Context, addr sdk.ConsAddress, infractionHeight, power int64, slashFactor sdk.Dec, infraction stakingtypes.Infraction) math.Int { - // fmt.Println("SLASHING", infraction) if infraction == stakingtypes.Infraction_INFRACTION_UNSPECIFIED { return math.NewInt(0) } @@ -123,7 +121,6 @@ func (k Keeper) SlashWithInfractionReason(ctx sdk.Context, addr sdk.ConsAddress, // If this is a previously standalone chain and infraction happened before the changeover was completed, // slash only on the standalone staking keeper. if k.IsPrevStandaloneChain(ctx) && infractionHeight < k.FirstConsumerHeight(ctx) { - fmt.Println("HAVE STANDALONE WITH INFRACTION", infraction) // NOTE: I'm not sure this code is 100% correct and it's relatively newly implemented // That is bothering me is that we call SlashWithInfractionReason without an infraction reason every time return k.standaloneStakingKeeper.SlashWithInfractionReason(ctx, addr, infractionHeight, power, slashFactor, stakingtypes.Infraction_INFRACTION_UNSPECIFIED) diff --git a/x/ccv/consumer/module.go b/x/ccv/consumer/module.go index b202cc1e61..605a9b2992 100644 --- a/x/ccv/consumer/module.go +++ b/x/ccv/consumer/module.go @@ -116,7 +116,6 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, data json.RawMessage) []abci.ValidatorUpdate { var genesisState consumertypes.GenesisState cdc.MustUnmarshalJSON(data, &genesisState) - // fmt.Println("CCCCC:// CONSUMER GENESIS STATE", string(data)) return am.keeper.InitGenesis(ctx, &genesisState) }