fix!: remove stopTime from MsgRemoveConsumer (#2208)

* init commit * renamed stop time to removal time * fix errors in tests * add GetAllConsumerWithIBCClients * use GetAllConsumerWithIBCClients for efficiency * fix UTs * remove GetAllLaunchedConsumerIds as not needed * typo in GetAllConsumersWithIBCClients * improve comment * Apply suggestions from code review Co-authored-by: Marius Poke <marius.poke@posteo.de> * took into account comments --------- Co-authored-by: mpoke <marius.poke@posteo.de>
cosmos · Sep 4, 2024 · 3cc9ba2 · 3cc9ba2
1 parent 7301916
commit 3cc9ba2
Show file tree

Hide file tree

Showing 43 changed files with 679 additions and 637 deletions.
diff --git a/proto/interchain_security/ccv/provider/v1/genesis.proto b/proto/interchain_security/ccv/provider/v1/genesis.proto
@@ -78,6 +78,8 @@ message ConsumerState {
   repeated interchain_security.ccv.v1.ValidatorSetChangePacketData
       pending_valset_changes = 6 [ (gogoproto.nullable) = false ];
   repeated string slash_downtime_ack = 7;
+  // the phase of the consumer chain
+  ConsumerPhase phase = 9;
 }
 
 // ValsetUpdateIdToHeight defines the genesis information for the mapping

diff --git a/proto/interchain_security/ccv/provider/v1/provider.proto b/proto/interchain_security/ccv/provider/v1/provider.proto
@@ -480,4 +480,6 @@ enum ConsumerPhase {
   CONSUMER_PHASE_LAUNCHED = 3;
   // STOPPED defines the phase in which a previously-launched chain has stopped.
   CONSUMER_PHASE_STOPPED = 4;
+  // DELETED defines the phase in which the state of a stopped chain has been deleted.
+  CONSUMER_PHASE_DELETED = 5;
 }
diff --git a/proto/interchain_security/ccv/provider/v1/query.proto b/proto/interchain_security/ccv/provider/v1/query.proto
@@ -156,7 +156,7 @@ message QueryConsumerGenesisResponse {
 
 message QueryConsumerChainsRequest {
   // The phase of the consumer chains returned (optional)
-  // Registered=1|Initialized=2|Launched=3|Stopped=4
+  // Registered=1|Initialized=2|Launched=3|Stopped=4|Deleted=5
   ConsumerPhase phase = 1;
   // The limit of consumer chains returned (optional)
   // default is 100

diff --git a/proto/interchain_security/ccv/provider/v1/tx.proto b/proto/interchain_security/ccv/provider/v1/tx.proto
@@ -228,12 +228,7 @@ message MsgRemoveConsumer {
 
   // the consumer id of the consumer chain to be stopped
   string consumer_id = 1;
-  // the time on the provider chain at which all validators are responsible to
-  // stop their consumer chain validator node
-  google.protobuf.Timestamp stop_time = 2
-      [ (gogoproto.stdtime) = true, (gogoproto.nullable) = false ];
-  // signer address
-  string signer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  string signer = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }
 
 // MsgRemoveConsumerResponse defines response type for MsgRemoveConsumer messages

diff --git a/tests/e2e/action_rapid_test.go b/tests/e2e/action_rapid_test.go
@@ -261,11 +261,10 @@ func GetSubmitConsumerAdditionProposalActionGen() *rapid.Generator[SubmitConsume
 func GetSubmitConsumerRemovalProposalActionGen() *rapid.Generator[SubmitConsumerRemovalProposalAction] {
 	return rapid.Custom(func(t *rapid.T) SubmitConsumerRemovalProposalAction {
 		return SubmitConsumerRemovalProposalAction{
-			Chain:          GetChainIDGen().Draw(t, "Chain"),
-			From:           GetValidatorIDGen().Draw(t, "From"),
-			Deposit:        rapid.Uint().Draw(t, "Deposit"),
-			ConsumerChain:  GetChainIDGen().Draw(t, "ConsumerChain"),
-			StopTimeOffset: time.Duration(rapid.Int64().Draw(t, "StopTimeOffset")),
+			Chain:         GetChainIDGen().Draw(t, "Chain"),
+			From:          GetValidatorIDGen().Draw(t, "From"),
+			Deposit:       rapid.Uint().Draw(t, "Deposit"),
+			ConsumerChain: GetChainIDGen().Draw(t, "ConsumerChain"),
 		}
 	})
 }

diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -740,11 +740,10 @@ func (tr Chain) submitConsumerAdditionLegacyProposal(
 }
 
 type SubmitConsumerRemovalProposalAction struct {
-	Chain          ChainID
-	From           ValidatorID
-	Deposit        uint
-	ConsumerChain  ChainID
-	StopTimeOffset time.Duration // offset from time.Now()
+	Chain         ChainID
+	From          ValidatorID
+	Deposit       uint
+	ConsumerChain ChainID
 }
 
 func (tr Chain) submitConsumerRemovalProposal(
@@ -762,7 +761,6 @@ func (tr Chain) submitConsumerRemovalProposal(
 
 	msg := types.MsgRemoveConsumer{
 		ConsumerId: consumerId,
-		StopTime:   tr.testConfig.containerConfig.Now.Add(action.StopTimeOffset),
 		Signer:     authority,
 	}
 
@@ -811,13 +809,11 @@ func (tr Chain) submitConsumerRemovalLegacyProposal(
 	action SubmitConsumerRemovalProposalAction,
 	verbose bool,
 ) {
-	stopTime := tr.testConfig.containerConfig.Now.Add(action.StopTimeOffset)
 	prop := client.ConsumerRemovalProposalJSON{
-		Title:    fmt.Sprintf("Stop the %v chain", action.ConsumerChain),
-		Summary:  "It was a great chain",
-		ChainId:  string(tr.testConfig.chainConfigs[action.ConsumerChain].ChainId),
-		StopTime: stopTime,
-		Deposit:  fmt.Sprint(action.Deposit) + `stake`,
+		Title:   fmt.Sprintf("Stop the %v chain", action.ConsumerChain),
+		Summary: "It was a great chain",
+		ChainId: string(tr.testConfig.chainConfigs[action.ConsumerChain].ChainId),
+		Deposit: fmt.Sprint(action.Deposit) + `stake`,
 	}
 
 	bz, err := json.Marshal(prop)

diff --git a/tests/e2e/state.go b/tests/e2e/state.go
@@ -525,7 +525,6 @@ func (tr Commands) GetProposal(chain ChainID, proposal uint) Proposal {
 		}
 	case "/interchain_security.ccv.provider.v1.MsgRemoveConsumer":
 		consumerId := rawContent.Get("consumer_id").String()
-		stopTime := rawContent.Get("stop_time").Time().Sub(tr.containerConfig.Now)
 
 		var chain ChainID
 		for i, conf := range tr.chainConfigs {
@@ -536,10 +535,9 @@ func (tr Commands) GetProposal(chain ChainID, proposal uint) Proposal {
 		}
 
 		return ConsumerRemovalProposal{
-			Deposit:  uint(deposit),
-			Status:   status,
-			Chain:    chain,
-			StopTime: int(stopTime.Milliseconds()),
+			Deposit: uint(deposit),
+			Status:  status,
+			Chain:   chain,
 		}
 	case "/ibc.applications.transfer.v1.MsgUpdateParams":
 		var params IBCTransferParams
@@ -757,7 +755,6 @@ func (tr Commands) GetIBCTransferParams(chain ChainID) IBCTransferParams {
 func (tr Commands) GetConsumerChains(chain ChainID) map[ChainID]bool {
 	binaryName := tr.chainConfigs[chain].BinaryName
 	cmd := tr.target.ExecCommand(binaryName,
-
 		"query", "provider", "list-consumer-chains",
 		`--node`, tr.GetQueryNode(chain),
 		`-o`, `json`,

diff --git a/tests/e2e/state_rapid_test.go b/tests/e2e/state_rapid_test.go
@@ -203,10 +203,9 @@ func GetConsumerAdditionProposalGen() *rapid.Generator[ConsumerAdditionProposal]
 func GetConsumerRemovalProposalGen() *rapid.Generator[ConsumerRemovalProposal] {
 	return rapid.Custom(func(t *rapid.T) ConsumerRemovalProposal {
 		return ConsumerRemovalProposal{
-			Deposit:  rapid.Uint().Draw(t, "Deposit"),
-			Chain:    GetChainIDGen().Draw(t, "Chain"),
-			StopTime: rapid.Int().Draw(t, "StopTime"),
-			Status:   rapid.String().Draw(t, "Status"),
+			Deposit: rapid.Uint().Draw(t, "Deposit"),
+			Chain:   GetChainIDGen().Draw(t, "Chain"),
+			Status:  rapid.String().Draw(t, "Status"),
 		}
 	})
 }

diff --git a/tests/e2e/steps_stop_chain.go b/tests/e2e/steps_stop_chain.go
@@ -1,10 +1,8 @@
 package main
 
 import (
-	"strconv"
-	"time"
-
 	gov "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+	"strconv"
 )
 
 // start relayer so that all messages are relayed
@@ -22,11 +20,10 @@ func stepsStopChain(consumerName string, propNumber uint) []Step {
 	s := []Step{
 		{
 			Action: SubmitConsumerRemovalProposalAction{
-				Chain:          ChainID("provi"),
-				From:           ValidatorID("bob"),
-				Deposit:        10000001,
-				ConsumerChain:  ChainID(consumerName),
-				StopTimeOffset: 0 * time.Millisecond,
+				Chain:         ChainID("provi"),
+				From:          ValidatorID("bob"),
+				Deposit:       10000001,
+				ConsumerChain: ChainID(consumerName),
 			},
 			State: State{
 				ChainID("provi"): ChainState{
@@ -35,10 +32,9 @@ func stepsStopChain(consumerName string, propNumber uint) []Step {
 					},
 					Proposals: &map[uint]Proposal{
 						propNumber: ConsumerRemovalProposal{
-							Deposit:  10000001,
-							Chain:    ChainID(consumerName),
-							StopTime: 0,
-							Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
+							Deposit: 10000001,
+							Chain:   ChainID(consumerName),
+							Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
 						},
 					},
 					ConsumerChains: &map[ChainID]bool{"consu": true}, // consumer chain not yet removed
@@ -56,10 +52,9 @@ func stepsStopChain(consumerName string, propNumber uint) []Step {
 				ChainID("provi"): ChainState{
 					Proposals: &map[uint]Proposal{
 						propNumber: ConsumerRemovalProposal{
-							Deposit:  10000001,
-							Chain:    ChainID(consumerName),
-							StopTime: 0,
-							Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_PASSED)),
+							Deposit: 10000001,
+							Chain:   ChainID(consumerName),
+							Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_PASSED)),
 						},
 					},
 					ValBalances: &map[ValidatorID]uint{
@@ -80,11 +75,10 @@ func stepsConsumerRemovalPropNotPassing(consumerName string, propNumber uint) []
 	s := []Step{
 		{
 			Action: SubmitConsumerRemovalProposalAction{
-				Chain:          ChainID("provi"),
-				From:           ValidatorID("bob"),
-				Deposit:        10000001,
-				ConsumerChain:  ChainID(consumerName),
-				StopTimeOffset: 0 * time.Millisecond,
+				Chain:         ChainID("provi"),
+				From:          ValidatorID("bob"),
+				Deposit:       10000001,
+				ConsumerChain: ChainID(consumerName),
 			},
 			State: State{
 				ChainID("provi"): ChainState{
@@ -93,10 +87,9 @@ func stepsConsumerRemovalPropNotPassing(consumerName string, propNumber uint) []
 					},
 					Proposals: &map[uint]Proposal{
 						propNumber: ConsumerRemovalProposal{
-							Deposit:  10000001,
-							Chain:    ChainID(consumerName),
-							StopTime: 0,
-							Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
+							Deposit: 10000001,
+							Chain:   ChainID(consumerName),
+							Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
 						},
 					},
 					ConsumerChains: &map[ChainID]bool{"consu": true}, // consumer chain not removed
@@ -114,10 +107,9 @@ func stepsConsumerRemovalPropNotPassing(consumerName string, propNumber uint) []
 				ChainID("provi"): ChainState{
 					Proposals: &map[uint]Proposal{
 						propNumber: ConsumerRemovalProposal{
-							Deposit:  10000001,
-							Chain:    ChainID(consumerName),
-							StopTime: 0,
-							Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_REJECTED)),
+							Deposit: 10000001,
+							Chain:   ChainID(consumerName),
+							Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_REJECTED)),
 						},
 					},
 					ValBalances: &map[ValidatorID]uint{

diff --git a/tests/e2e/testlib/types.go b/tests/e2e/testlib/types.go
@@ -306,10 +306,9 @@ func (p UpgradeProposal) isProposal() {}
 func (p ConsumerAdditionProposal) isProposal() {}
 
 type ConsumerRemovalProposal struct {
-	Deposit  uint
-	Chain    ChainID
-	StopTime int
-	Status   string
+	Deposit uint
+	Chain   ChainID
+	Status  string
 }
 
 func (p ConsumerRemovalProposal) isProposal() {}

diff --git a/tests/e2e/trace_handlers_test.go b/tests/e2e/trace_handlers_test.go
@@ -27,10 +27,9 @@ var proposalInStateSteps = []Step{
 			ChainID("provi"): ChainState{
 				Proposals: &map[uint]Proposal{
 					1: ConsumerRemovalProposal{
-						Deposit:  10000001,
-						Chain:    ChainID("foo"),
-						StopTime: 0,
-						Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
+						Deposit: 10000001,
+						Chain:   ChainID("foo"),
+						Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
 					},
 				},
 			},
@@ -136,10 +135,9 @@ func TestMarshalAndUnmarshalChainState(t *testing.T) {
 		"consumer removal proposal": {ChainState{
 			Proposals: &map[uint]Proposal{
 				5: ConsumerRemovalProposal{
-					Deposit:  10000001,
-					Chain:    ChainID("test123"),
-					StopTime: 5000000000,
-					Status:   strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_PASSED)),
+					Deposit: 10000001,
+					Chain:   ChainID("test123"),
+					Status:  strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_PASSED)),
 				},
 			},
 			ValBalances: &map[ValidatorID]uint{

diff --git a/tests/e2e/v4/state.go b/tests/e2e/v4/state.go
@@ -317,7 +317,6 @@ func (tr Commands) GetProposal(chain ChainID, proposal uint) Proposal {
 		}
 	case "/interchain_security.ccv.provider.v1.ConsumerRemovalProposal":
 		chainId := gjson.Get(string(bz), `messages.0.content.chain_id`).String()
-		stopTime := gjson.Get(string(bz), `messages.0.content.stop_time`).Time().Sub(containerConfig.Now)
 
 		var chain ChainID
 		for i, conf := range chainConfigs {
@@ -328,10 +327,9 @@ func (tr Commands) GetProposal(chain ChainID, proposal uint) Proposal {
 		}
 
 		return ConsumerRemovalProposal{
-			Deposit:  uint(deposit),
-			Status:   status,
-			Chain:    chain,
-			StopTime: int(stopTime.Milliseconds()),
+			Deposit: uint(deposit),
+			Status:  status,
+			Chain:   chain,
 		}
 	case "/cosmos.params.v1beta1.ParameterChangeProposal":
 		return ParamsProposal{
@@ -391,7 +389,6 @@ func (tr Commands) GetParam(chain ChainID, param Param) string {
 func (tr Commands) GetConsumerChains(chain ChainID) map[ChainID]bool {
 	binaryName := tr.ChainConfigs[chain].BinaryName
 	cmd := tr.Target.ExecCommand(binaryName,
-
 		"query", "provider", "list-consumer-chains",
 		`--node`, tr.GetQueryNode(chain),
 		`-o`, `json`,

diff --git a/tests/integration/stop_consumer.go b/tests/integration/stop_consumer.go
@@ -3,6 +3,7 @@ package integration
 import (
 	"cosmossdk.io/math"
 	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
+	"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
@@ -88,7 +89,8 @@ func (s *CCVTestSuite) TestStopConsumerChain() {
 	}
 
 	// stop the consumer chain
-	err = providerKeeper.StopConsumerChain(s.providerCtx(), firstBundle.ConsumerId, true)
+	providerKeeper.SetConsumerPhase(s.providerCtx(), firstBundle.ConsumerId, types.ConsumerPhase_CONSUMER_PHASE_STOPPED)
+	err = providerKeeper.DeleteConsumerChain(s.providerCtx(), firstBundle.ConsumerId)
 	s.Require().NoError(err)
 
 	// check all states are removed and the unbonding operation released
@@ -105,7 +107,8 @@ func (s *CCVTestSuite) TestStopConsumerOnChannelClosed() {
 	providerKeeper := s.providerApp.GetProviderKeeper()
 
 	// stop the consumer chain
-	err := providerKeeper.StopConsumerChain(s.providerCtx(), s.getFirstBundle().ConsumerId, true)
+	providerKeeper.SetConsumerPhase(s.providerCtx(), s.getFirstBundle().ConsumerId, types.ConsumerPhase_CONSUMER_PHASE_STOPPED)
+	err := providerKeeper.DeleteConsumerChain(s.providerCtx(), s.getFirstBundle().ConsumerId)
 	s.Require().NoError(err)
 
 	err = s.path.EndpointA.UpdateClient()

diff --git a/testutil/keeper/expectations.go b/testutil/keeper/expectations.go
@@ -80,9 +80,8 @@ func GetMocksForSetConsumerChain(ctx sdk.Context, mocks *MockedKeepers,
 	}
 }
 
-// GetMocksForStopConsumerChainWithCloseChannel returns mock expectations needed to call StopConsumerChain() when
-// `closeChan` is true.
-func GetMocksForStopConsumerChainWithCloseChannel(ctx sdk.Context, mocks *MockedKeepers) []*gomock.Call {
+// GetMocksForDeleteConsumerChain returns mock expectations needed to call `DeleteConsumerChain`
+func GetMocksForDeleteConsumerChain(ctx sdk.Context, mocks *MockedKeepers) []*gomock.Call {
 	dummyCap := &capabilitytypes.Capability{}
 	return []*gomock.Call{
 		mocks.MockChannelKeeper.EXPECT().GetChannel(gomock.Any(), types.ProviderPortID, "channelID").Return(