Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: remove stopTime from MsgRemoveConsumer #2208

Merged
merged 14 commits into from
Sep 4, 2024
2 changes: 2 additions & 0 deletions proto/interchain_security/ccv/provider/v1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
mpoke marked this conversation as resolved.
Show resolved Hide resolved
}

// ValsetUpdateIdToHeight defines the genesis information for the mapping
Expand Down
2 changes: 2 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion proto/interchain_security/ccv/provider/v1/query.proto
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
syntax = "proto3";

Check failure on line 1 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present message "QueryAllPairsValConAddrByConsumerChainIDRequest" was deleted from file.

Check failure on line 1 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present message "QueryAllPairsValConAddrByConsumerChainIDResponse" was deleted from file.
package interchain_security.ccv.provider.v1;

option go_package = "github.com/cosmos/interchain-security/v5/x/ccv/provider/types";
Expand All @@ -13,7 +13,7 @@
import "cosmos_proto/cosmos.proto";
import "cosmos/staking/v1beta1/staking.proto";

service Query {

Check failure on line 16 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present RPC "QueryAllPairsValConAddrByConsumerChainID" on service "Query" was deleted.
// ConsumerGenesis queries the genesis state needed to start a consumer chain
// whose proposal has been accepted
rpc QueryConsumerGenesis(QueryConsumerGenesisRequest)
Expand Down Expand Up @@ -156,7 +156,7 @@

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
Expand All @@ -183,7 +183,7 @@
// Corresponds to a list of provider consensus addresses of validators that CANNOT validate the consumer chain.
repeated string denylist = 8;
// The phase the consumer chain (Registered=0|Initialized=1|FailedToLaunch=2|Launched=3|Stopped=4)
string phase = 9;

Check failure on line 186 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "9" with name "phase" on message "Chain" changed type from "enum" to "string".
// The metadata of the consumer chain
ConsumerMetadata metadata = 10 [(gogoproto.nullable) = false ];
// Corresponds to the minimal amount of (provider chain) stake required to validate on the consumer chain.
Expand Down Expand Up @@ -332,7 +332,7 @@
}

message QueryConsumerChainsValidatorHasToValidateResponse {
repeated string consumer_ids = 1;

Check failure on line 335 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "1" with name "consumer_ids" on message "QueryConsumerChainsValidatorHasToValidateResponse" changed option "json_name" from "consumerChainIds" to "consumerIds".

Check failure on line 335 in proto/interchain_security/ccv/provider/v1/query.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "1" on message "QueryConsumerChainsValidatorHasToValidateResponse" changed name from "consumer_chain_ids" to "consumer_ids".
}

message QueryValidatorConsumerCommissionRateRequest {
Expand Down
7 changes: 1 addition & 6 deletions proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,12 @@

// MsgRemoveConsumer defines the message used to remove (and stop) a consumer chain.
// If it passes, all the consumer chain's state is eventually removed from the provider chain.
message MsgRemoveConsumer {

Check failure on line 226 in proto/interchain_security/ccv/provider/v1/tx.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "3" with name "signer" on message "MsgRemoveConsumer" was deleted.
option (cosmos.msg.v1.signer) = "signer";

// 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"];

Check failure on line 231 in proto/interchain_security/ccv/provider/v1/tx.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "2" with name "signer" on message "MsgRemoveConsumer" changed cardinality from "optional with explicit presence" to "optional with implicit presence".

Check failure on line 231 in proto/interchain_security/ccv/provider/v1/tx.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "2" with name "signer" on message "MsgRemoveConsumer" changed option "json_name" from "stopTime" to "signer".

Check failure on line 231 in proto/interchain_security/ccv/provider/v1/tx.proto

View workflow job for this annotation

GitHub Actions / break-check

Field "2" with name "signer" on message "MsgRemoveConsumer" changed type from "message" to "string".
}

// MsgRemoveConsumerResponse defines response type for MsgRemoveConsumer messages
Expand Down
9 changes: 4 additions & 5 deletions tests/e2e/action_rapid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
})
}
Expand Down
20 changes: 8 additions & 12 deletions tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -762,7 +761,6 @@ func (tr Chain) submitConsumerRemovalProposal(

msg := types.MsgRemoveConsumer{
ConsumerId: consumerId,
StopTime: tr.testConfig.containerConfig.Now.Add(action.StopTimeOffset),
Signer: authority,
}

Expand Down Expand Up @@ -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)
Expand Down
9 changes: 3 additions & 6 deletions tests/e2e/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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`,
Expand Down
7 changes: 3 additions & 4 deletions tests/e2e/state_rapid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
})
}
Expand Down
50 changes: 21 additions & 29 deletions tests/e2e/steps_stop_chain.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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{
Expand All @@ -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
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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
Expand All @@ -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{
Expand Down
7 changes: 3 additions & 4 deletions tests/e2e/testlib/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down
14 changes: 6 additions & 8 deletions tests/e2e/trace_handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
},
},
},
Expand Down Expand Up @@ -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{
Expand Down
9 changes: 3 additions & 6 deletions tests/e2e/v4/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{
Expand Down Expand Up @@ -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`,
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/stop_consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)
Comment on lines +110 to +111
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complete the test function.

The changes made to stop the consumer chain using the new methods SetConsumerPhase and DeleteConsumerChain are in line with the PR objectives. However, the test function is incomplete and has commented out code that needs to be implemented.

Please complete the test function by implementing the following:

  1. Simulate the relayer behavior by calling OnChanCloseConfirm on the provider chain's channel end.
  2. Check for the panic in the consumer chain's BeginBlock.
  3. Check if the provider's channel is removed.

Do you want me to open a GitHub issue to track this task?

s.Require().NoError(err)

err = s.path.EndpointA.UpdateClient()
Expand Down
5 changes: 2 additions & 3 deletions testutil/keeper/expectations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading
Loading