diff --git a/tests/integration/actions.go b/tests/integration/actions.go index beec0b1f08..379488ab34 100644 --- a/tests/integration/actions.go +++ b/tests/integration/actions.go @@ -597,7 +597,7 @@ func (tr TestRun) addChainToRelayer( keyName := "query" rpcAddr := "http://" + queryNodeIP + ":26658" grpcAddr := "tcp://" + queryNodeIP + ":9091" - wsAddr := "ws://" + queryNodeIP + ":26657/websocket" + wsAddr := "ws://" + queryNodeIP + ":26658/websocket" chainConfig := fmt.Sprintf(hermesChainConfigTemplate, grpcAddr, @@ -693,6 +693,28 @@ type addIbcChannelAction struct { order string } +type startHermesAction struct { +} + +func (tr TestRun) startHermes( + action startHermesAction, + verbose bool, +) { + // hermes start is running in detached mode + //#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments. + cmd := exec.Command("docker", "exec", "-d", tr.containerConfig.instanceName, "hermes", + "start", + ) + + if err := cmd.Start(); err != nil { + log.Fatal(err) + } + + if verbose { + fmt.Println("started Hermes") + } +} + func (tr TestRun) addIbcChannel( action addIbcChannelAction, verbose bool, diff --git a/tests/integration/main.go b/tests/integration/main.go index 96cc1d28e8..82e457a3bc 100644 --- a/tests/integration/main.go +++ b/tests/integration/main.go @@ -131,6 +131,8 @@ func (tr *TestRun) runStep(step Step, verbose bool) { tr.assignConsumerPubKey(action, verbose) case slashThrottleDequeue: tr.waitForSlashThrottleDequeue(action, verbose) + case startHermesAction: + tr.startHermes(action, verbose) default: log.Fatalf("unknown action in testRun %s: %#v", tr.name, action) } diff --git a/tests/integration/steps.go b/tests/integration/steps.go index de91a744f2..448b6d3bce 100644 --- a/tests/integration/steps.go +++ b/tests/integration/steps.go @@ -23,7 +23,9 @@ var happyPathSteps = concatSteps( stepsRejectEquivocationProposal("consu", 2), // prop to tombstone bob is rejected stepsDoubleSignOnProviderAndConsumer("consu"), // carol double signs on provider, bob double signs on consumer stepsSubmitEquivocationProposal("consu", 2), // now prop to tombstone bob is submitted and accepted - stepsStopChain("consu", 3), + stepsStartHermes(), + stepsConsumerRemovalPropNotPassing("consu", 3), // submit removal prop but vote no on it - chain should stay + stepsStopChain("consu", 4), // stop chain ) var slashThrottleSteps = concatSteps( diff --git a/tests/integration/steps_stop_chain.go b/tests/integration/steps_stop_chain.go index e231592d76..5884afe9f8 100644 --- a/tests/integration/steps_stop_chain.go +++ b/tests/integration/steps_stop_chain.go @@ -2,6 +2,16 @@ package main import "time" +// start hermes so that all messages are relayed +func stepsStartHermes() []Step { + return []Step{ + { + action: startHermesAction{}, + state: State{}, + }, + } +} + // submits a consumer-removal proposal and removes the chain func stepsStopChain(consumerName string, propNumber uint) []Step { s := []Step{ @@ -58,3 +68,62 @@ func stepsStopChain(consumerName string, propNumber uint) []Step { return s } + +// submits a consumer-removal proposal and votes no on it +// the chain should not be removed +func stepsConsumerRemovalPropNotPassing(consumerName string, propNumber uint) []Step { + s := []Step{ + + { + action: submitConsumerRemovalProposalAction{ + chain: chainID("provi"), + from: validatorID("bob"), + deposit: 10000001, + consumerChain: chainID(consumerName), + stopTimeOffset: 0 * time.Millisecond, + }, + state: State{ + chainID("provi"): ChainState{ + ValBalances: &map[validatorID]uint{ + validatorID("bob"): 9489999999, + }, + Proposals: &map[uint]Proposal{ + propNumber: ConsumerRemovalProposal{ + Deposit: 10000001, + Chain: chainID(consumerName), + StopTime: 0, + Status: "PROPOSAL_STATUS_VOTING_PERIOD", + }, + }, + ConsumerChains: &map[chainID]bool{"consu": true}, // consumer chain not removed + }, + }, + }, + { + action: voteGovProposalAction{ + chain: chainID("provi"), + from: []validatorID{validatorID("alice"), validatorID("bob"), validatorID("carol")}, + vote: []string{"no", "no", "no"}, + propNumber: propNumber, + }, + state: State{ + chainID("provi"): ChainState{ + Proposals: &map[uint]Proposal{ + propNumber: ConsumerRemovalProposal{ + Deposit: 10000001, + Chain: chainID(consumerName), + StopTime: 0, + Status: "PROPOSAL_STATUS_REJECTED", + }, + }, + ValBalances: &map[validatorID]uint{ + validatorID("bob"): 9500000000, + }, + ConsumerChains: &map[chainID]bool{"consu": true}, // consumer chain not removed + }, + }, + }, + } + + return s +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index 30f0c6b12e..e3d1541f98 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -598,7 +598,7 @@ func (k Keeper) CreateConsumerClientInCachedCtx(ctx sdk.Context, p types.Consume // from a given consumer removal proposal in a cached context func (k Keeper) StopConsumerChainInCachedCtx(ctx sdk.Context, p types.ConsumerRemovalProposal) (cc sdk.Context, writeCache func(), err error) { cc, writeCache = ctx.CacheContext() - err = k.StopConsumerChain(ctx, p.ChainId, true) + err = k.StopConsumerChain(cc, p.ChainId, true) return } diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 6c576f38df..9e9580e6b9 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -383,7 +383,7 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { type testCase struct { description string - malleate func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) + setupMocks func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) // Consumer removal proposal to handle prop *providertypes.ConsumerRemovalProposal @@ -392,16 +392,21 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { // Whether it's expected that the proposal is successfully verified // and appended to the pending proposals expAppendProp bool + + // chainID of the consumer chain + // tests need to check that the CCV channel is not closed prematurely + chainId string } // Snapshot times asserted in tests now := time.Now().UTC() - hourFromNow := now.Add(time.Hour).UTC() + hourAfterNow := now.Add(time.Hour).UTC() + hourBeforeNow := now.Add(-time.Hour).UTC() tests := []testCase{ { description: "valid proposal", - malleate: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) { + setupMocks: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) { k.SetConsumerClientId(ctx, chainID, "ClientID") }, prop: providertypes.NewConsumerRemovalProposal( @@ -410,20 +415,52 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { "chainID", now, ).(*providertypes.ConsumerRemovalProposal), - blockTime: hourFromNow, // After stop time. + blockTime: hourAfterNow, // After stop time. expAppendProp: true, + chainId: "chainID", }, { - description: "invalid valid proposal: consumer chain does not exist", - malleate: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) {}, + description: "valid proposal - stop_time in the past", + setupMocks: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) { + k.SetConsumerClientId(ctx, chainID, "ClientID") + }, + prop: providertypes.NewConsumerRemovalProposal( + "title", + "description", + "chainID", + hourBeforeNow, + ).(*providertypes.ConsumerRemovalProposal), + blockTime: hourAfterNow, // After stop time. + expAppendProp: true, + chainId: "chainID", + }, + { + description: "valid proposal - before stop_time in the future", + setupMocks: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) { + k.SetConsumerClientId(ctx, chainID, "ClientID") + }, + prop: providertypes.NewConsumerRemovalProposal( + "title", + "description", + "chainID", + hourAfterNow, + ).(*providertypes.ConsumerRemovalProposal), + blockTime: now, + expAppendProp: true, + chainId: "chainID", + }, + { + description: "rejected valid proposal - consumer chain does not exist", + setupMocks: func(ctx sdk.Context, k providerkeeper.Keeper, chainID string) {}, prop: providertypes.NewConsumerRemovalProposal( "title", "description", "chainID-2", - hourFromNow, + hourAfterNow, ).(*providertypes.ConsumerRemovalProposal), - blockTime: hourFromNow, // After stop time. + blockTime: hourAfterNow, // After stop time. expAppendProp: false, + chainId: "chainID-2", }, } @@ -436,13 +473,13 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { ctx = ctx.WithBlockTime(tc.blockTime) // Mock expectations and setup for stopping the consumer chain, if applicable + // Note: when expAppendProp is false, no mocks are setup, + // meaning no external keeper methods are allowed to be called. if tc.expAppendProp { testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks) } - // Note: when expAppendProp is false, no mocks are setup, - // meaning no external keeper methods are allowed to be called. - tc.malleate(ctx, providerKeeper, tc.prop.ChainId) + tc.setupMocks(ctx, providerKeeper, tc.prop.ChainId) err := providerKeeper.HandleConsumerRemovalProposal(ctx, tc.prop) @@ -453,6 +490,9 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { found := providerKeeper.PendingConsumerRemovalPropExists(ctx, tc.prop.ChainId, tc.prop.StopTime) require.True(t, found) + // confirm that the channel was not closed + _, found = providerKeeper.GetChainToChannel(ctx, tc.chainId) + require.True(t, found) } else { require.Error(t, err)