From 14129233594bd72e57fa5ac286c79d6c420cce2a Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 12 Jul 2023 07:59:13 -0700 Subject: [PATCH] feat: remove consumer panic when ccv channel is closed (#1127) * rm panic * rm tests * Update CHANGELOG.md * update comment --- CHANGELOG.md | 8 ++++++++ tests/integration/stop_consumer.go | 29 ----------------------------- testutil/integration/debug_test.go | 4 ---- x/ccv/consumer/module.go | 5 +---- 4 files changed, 9 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92dc3b5efb..b984d03c08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ Add an entry to the unreleased section whenever merging a PR to main that is not * (fix) [#720](https://github.com/cosmos/interchain-security/issues/720) Fix the attribute `AttributeDistributionTotal` value in `FeeDistribution` event emit. * (deps) [#1119](https://github.com/cosmos/interchain-security/pull/1119) bump cometbft from `v0.37.1` to `0.37.2`. +## v3.1.0 + +Date July 11th, 2023 + +A minor upgrade to v3.0.0, which removes the panic in the consumer ccv module which would occur in an emergency scenario where the ccv channel is closed. + +* (feat) [#1127](https://github.com/cosmos/interchain-security/pull/1127) Remove consumer panic when ccv channel is closed + ## v3.0.0 Date: June 21st, 2023 diff --git a/tests/integration/stop_consumer.go b/tests/integration/stop_consumer.go index 43ecaf9e33..b3d32ee6a4 100644 --- a/tests/integration/stop_consumer.go +++ b/tests/integration/stop_consumer.go @@ -6,8 +6,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ccv "github.com/cosmos/interchain-security/v3/x/ccv/types" ) @@ -213,30 +211,3 @@ func (s *CCVTestSuite) checkConsumerChainIsRemoved(chainID string, checkChannel s.Require().Empty(slashData) s.Require().Empty(vscMaturedData) } - -// TestProviderChannelClosed checks that a consumer chain panics -// when the provider channel was established and then closed -func (suite *CCVTestSuite) TestProviderChannelClosed() { - suite.SetupCCVChannel(suite.path) - // establish provider channel with a first VSC packet - suite.SendEmptyVSCPacket() - - consumerKeeper := suite.consumerApp.GetConsumerKeeper() - - channelID, found := consumerKeeper.GetProviderChannel(suite.consumerChain.GetContext()) - suite.Require().True(found) - - // close provider channel - err := consumerKeeper.ChanCloseInit(suite.consumerChain.GetContext(), ccv.ConsumerPortID, channelID) - suite.Require().NoError(err) - suite.Require().True(consumerKeeper.IsChannelClosed(suite.consumerChain.GetContext(), channelID)) - - // assert begin blocker did panics - defer func() { - if r := recover(); r != nil { - return - } - suite.Require().Fail("Begin blocker did not panic with a closed channel") - }() - suite.consumerApp.BeginBlocker(suite.consumerChain.GetContext(), abci.RequestBeginBlock{}) -} diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go index 3d04c540ee..077f33cde3 100644 --- a/testutil/integration/debug_test.go +++ b/testutil/integration/debug_test.go @@ -157,10 +157,6 @@ func TestStopConsumerOnChannelClosed(t *testing.T) { runCCVTestByName(t, "TestStopConsumerOnChannelClosed") } -func TestProviderChannelClosed(t *testing.T) { - runCCVTestByName(t, "TestProviderChannelClosed") -} - // // Throttle tests // diff --git a/x/ccv/consumer/module.go b/x/ccv/consumer/module.go index 6c05273451..fe9b18a945 100644 --- a/x/ccv/consumer/module.go +++ b/x/ccv/consumer/module.go @@ -143,12 +143,9 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { channelID, found := am.keeper.GetProviderChannel(ctx) if found && am.keeper.IsChannelClosed(ctx, channelID) { // The CCV channel was established, but it was then closed; - // the consumer chain is no longer safe, thus it MUST shut down. - // This is achieved by panicking, similar as it's done in the - // x/upgrade module of cosmos-sdk. + // the consumer chain is not secured anymore, but we allow it to run as a POA chain and log an error. channelClosedMsg := fmt.Sprintf("CCV channel %q was closed - shutdown consumer chain since it is not secured anymore", channelID) am.keeper.Logger(ctx).Error(channelClosedMsg) - panic(channelClosedMsg) } // map next block height to the vscID of the current block height