Skip to content

Commit

Permalink
Cleanup states after consumer chain removal (#268)
Browse files Browse the repository at this point in the history
* * Remove provider client ID from consumer chain before a shutdown due to timeout

* Remove PendingVSCPackets from provider states after a consumer chain removal

* remove consumer genesis from provider states after removal

* Update x/ccv/consumer/module.go

Co-authored-by: Marius Poke <[email protected]>

* make pass the tests

* comment tests

* fix error check in tests

* fix nits

* * remove consumer delete operations before shutdown

* move unbonding op indexes deletion outside of the iterator in stop consumer

* remove unused iterators

Co-authored-by: Marius Poke <[email protected]>
  • Loading branch information
sainoe and mpoke authored Sep 21, 2022
1 parent f9e704a commit 12ac66c
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 28 deletions.
2 changes: 1 addition & 1 deletion app/consumer/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func New(
app.GetSubspace(slashingtypes.ModuleName),
)

// register slashing module StakingHooks to the consumer keeper
// register slashing module Slashing hooks to the consumer keeper
app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks())
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper)

Expand Down
37 changes: 33 additions & 4 deletions tests/e2e/stop_consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
appConsumer "github.com/cosmos/interchain-security/app/consumer"
appProvider "github.com/cosmos/interchain-security/app/provider"
providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
Expand Down Expand Up @@ -77,6 +78,7 @@ func (s *ProviderTestSuite) TestStopConsumerChain() {
func(suite *ProviderTestSuite) error {
s.providerChain.App.(*appProvider.App).ProviderKeeper.SetSlashAcks(s.providerCtx(), consumerChainID, []string{"validator-1", "validator-2", "validator-3"})
s.providerChain.App.(*appProvider.App).ProviderKeeper.SetLockUnbondingOnTimeout(s.providerCtx(), consumerChainID)
s.providerChain.App.(*appProvider.App).ProviderKeeper.AppendPendingVSC(s.providerCtx(), consumerChainID, ccv.ValidatorSetChangePacketData{ValsetUpdateId: 1})
return nil
},
},
Expand Down Expand Up @@ -247,8 +249,10 @@ func (s *ProviderTestSuite) checkConsumerChainIsRemoved(chainID string, lockUbd
}

// verify consumer chain's states are removed
_, found := providerKeeper.GetConsumerGenesis(s.providerCtx(), chainID)
s.Require().False(found)
s.Require().False(providerKeeper.GetLockUnbondingOnTimeout(s.providerCtx(), chainID))
_, found := providerKeeper.GetConsumerClientId(s.providerCtx(), chainID)
_, found = providerKeeper.GetConsumerClientId(s.providerCtx(), chainID)
s.Require().False(found)

_, found = providerKeeper.GetChainToChannel(s.providerCtx(), chainID)
Expand All @@ -259,12 +263,11 @@ func (s *ProviderTestSuite) checkConsumerChainIsRemoved(chainID string, lockUbd

s.Require().Nil(providerKeeper.GetSlashAcks(s.providerCtx(), chainID))
s.Require().Zero(providerKeeper.GetInitChainHeight(s.providerCtx(), chainID))
// TODO Simon: check that pendingVSCPacket are emptied - once
// https://github.com/cosmos/interchain-security/issues/27 is implemented
s.Require().Nil(providerKeeper.GetPendingVSCs(s.providerCtx(), chainID))
}

// TODO Simon: duplicated from consumer/keeper_test.go; figure out how it can be refactored
// SendEmptyVSCPacket sends a VSC packet without any changes
// SendEmptyVSCPacket sends a VSC packet with no valset changes
// to ensure that the CCV channel gets established
func (s *ProviderTestSuite) SendEmptyVSCPacket() {
providerKeeper := s.providerChain.App.(*appProvider.App).ProviderKeeper
Expand Down Expand Up @@ -292,3 +295,29 @@ func (s *ProviderTestSuite) SendEmptyVSCPacket() {
err = s.path.EndpointA.RecvPacket(packet)
s.Require().NoError(err)
}

// TestProviderChannelClosed checks that a consumer chain panics
// when the provider channel was established and then closed
func (suite *ConsumerKeeperTestSuite) TestProviderChannelClosed() {

suite.SetupCCVChannel()
// establish provider channel with a first VSC packet
suite.SendEmptyVSCPacket()

channelID, found := suite.consumerChain.App.(*appConsumer.App).ConsumerKeeper.GetProviderChannel(suite.consumerChain.GetContext())
suite.Require().True(found)

// close provider channel
err := suite.consumerChain.App.(*appConsumer.App).ConsumerKeeper.ChanCloseInit(suite.consumerChain.GetContext(), ccv.ConsumerPortID, channelID)
suite.Require().NoError(err)
suite.Require().True(suite.consumerChain.App.(*appConsumer.App).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.consumerChain.App.(*appConsumer.App).BeginBlocker(suite.consumerChain.GetContext(), abci.RequestBeginBlock{})
}
20 changes: 10 additions & 10 deletions x/ccv/consumer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (k Keeper) GetPacketMaturityTime(ctx sdk.Context, vscId uint64) uint64 {
return binary.BigEndian.Uint64(bz)
}

// DeletePacketMaturityTime deletes the the maturity time for a given received VSC packet id
// DeletePacketMaturityTime deletes the packet maturity time for a given received VSC packet id
func (k Keeper) DeletePacketMaturityTime(ctx sdk.Context, vscId uint64) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.PacketMaturityTimeKey(vscId))
Expand Down Expand Up @@ -334,9 +334,9 @@ func (k Keeper) SetOutstandingDowntime(ctx sdk.Context, address sdk.ConsAddress)
store.Set(types.OutstandingDowntimeKey(address), []byte{})
}

// ClearOutstandingDowntime clears the outstanding downtime flag for a given validator
func (k Keeper) ClearOutstandingDowntime(ctx sdk.Context, address string) {
consAddr, err := sdk.ConsAddressFromBech32(address)
// DeleteOutstandingDowntime deletes the outstanding downtime flag for the given validator consensus address
func (k Keeper) DeleteOutstandingDowntime(ctx sdk.Context, consAddress string) {
consAddr, err := sdk.ConsAddressFromBech32(consAddress)
if err != nil {
return
}
Expand Down Expand Up @@ -417,15 +417,15 @@ func (k Keeper) GetPendingSlashRequests(ctx sdk.Context) []*types.SlashRequest {
return sr.GetRequests()
}

// ClearPendingSlashRequests clears the pending slash requests in store
func (k Keeper) DeletePendingSlashRequests(ctx sdk.Context) {
store := ctx.KVStore(k.storeKey)
store.Delete([]byte{types.PendingSlashRequestsBytePrefix})
}

// AppendPendingSlashRequests appends the given slash request to the pending slash requests in store
func (k Keeper) AppendPendingSlashRequests(ctx sdk.Context, req types.SlashRequest) {
requests := k.GetPendingSlashRequests(ctx)
requests = append(requests, &req)
k.SetPendingSlashRequests(ctx, requests)
}

// ClearPendingSlashRequests clears the pending slash requests in store
func (k Keeper) ClearPendingSlashRequests(ctx sdk.Context) {
store := ctx.KVStore(k.storeKey)
store.Delete([]byte{types.PendingSlashRequestsBytePrefix})
}
2 changes: 1 addition & 1 deletion x/ccv/consumer/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func TestPendingSlashRequests(t *testing.T) {
operation: func() { consumerKeeper.AppendPendingSlashRequests(ctx, types.SlashRequest{}) },
expLen: 11,
}, {
operation: func() { consumerKeeper.ClearPendingSlashRequests(ctx) },
operation: func() { consumerKeeper.DeletePendingSlashRequests(ctx) },
expLen: 0,
},
}
Expand Down
7 changes: 4 additions & 3 deletions x/ccv/consumer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ func (k Keeper) OnRecvVSCPacket(ctx sdk.Context, packet channeltypes.Packet, new
// set height to VSC id mapping
k.SetHeightValsetUpdateID(ctx, uint64(ctx.BlockHeight())+1, newChanges.ValsetUpdateId)

// set outstanding slashing flags to false
// remove outstanding slashing flags of the validators
// for which the slashing was acknowledged by the provider chain
for _, addr := range newChanges.GetSlashAcks() {
k.ClearOutstandingDowntime(ctx, addr)
k.DeleteOutstandingDowntime(ctx, addr)
}

ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
Expand Down Expand Up @@ -201,7 +202,7 @@ func (k Keeper) SendPendingSlashRequests(ctx sdk.Context) {
}

// clear pending slash requests
k.ClearPendingSlashRequests(ctx)
k.DeletePendingSlashRequests(ctx)
}

// OnAcknowledgementPacket executes application logic for acknowledgments of sent VSCMatured and Slash packets
Expand Down
3 changes: 0 additions & 3 deletions x/ccv/consumer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
// the CCV channel was established, but it was then closed;
// the consumer chain is no longer safe

// cleanup state
am.keeper.DeleteProviderChannel(ctx)

channelClosedMsg := fmt.Sprintf("CCV channel %q was closed - shutdown consumer chain since it is not secured anymore", channelID)
ctx.Logger().Error(channelClosedMsg)
panic(channelClosedMsg)
Expand Down
5 changes: 2 additions & 3 deletions x/ccv/consumer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/binary"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
)

const (
Expand Down Expand Up @@ -122,8 +121,8 @@ func HeightValsetUpdateIDKey(height uint64) []byte {
}

// OutstandingDowntimeKey returns the key to a validators' outstanding downtime by consensus address
func OutstandingDowntimeKey(v sdk.ConsAddress) []byte {
return append([]byte{OutstandingDowntimeBytePrefix}, address.MustLengthPrefix(v.Bytes())...)
func OutstandingDowntimeKey(address sdk.ConsAddress) []byte {
return append([]byte{OutstandingDowntimeBytePrefix}, address.Bytes()...)
}

// CrossChainValidatorKey returns the key to a cross chain validator by consensus address
Expand Down
5 changes: 5 additions & 0 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ func (k Keeper) GetConsumerGenesis(ctx sdk.Context, chainID string) (consumertyp
return data, true
}

func (k Keeper) DeleteConsumerGenesis(ctx sdk.Context, chainID string) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.ConsumerGenesisKey(chainID))
}

// VerifyConsumerChain verifies that the chain trying to connect on the channel handshake
// is the expected consumer chain.
func (k Keeper) VerifyConsumerChain(ctx sdk.Context, channelID string, connectionHops []string) error {
Expand Down
14 changes: 11 additions & 3 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, clos

// clean up states
k.DeleteConsumerClientId(ctx, chainID)
k.DeleteConsumerGenesis(ctx, chainID)
k.DeleteLockUnbondingOnTimeout(ctx, chainID)

// close channel and delete the mappings between chain ID and channel ID
Expand All @@ -71,11 +72,12 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, clos
k.DeleteChannelToChain(ctx, channelID)
}

// TODO remove pending VSC packets once https://github.com/cosmos/interchain-security/issues/27 is fixed
k.DeleteInitChainHeight(ctx, chainID)
k.EmptySlashAcks(ctx, chainID)
k.EmptyPendingVSC(ctx, chainID)

// release unbonding operations if they aren't locked
var vscIDs []uint64
if !lockUbd {
// iterate over the consumer chain's unbonding operation VSC ids
k.IterateOverUnbondingOpIndex(ctx, chainID, func(vscID uint64, ids []uint64) bool {
Expand Down Expand Up @@ -105,15 +107,21 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, lockUbd, clos
if err := k.AppendMaturedUnbondingOps(ctx, maturedIds); err != nil {
panic(fmt.Errorf("mature unbonding ops could not be appended: %w", err))
}
// clean up index
k.DeleteUnbondingOpIndex(ctx, chainID, vscID)

vscIDs = append(vscIDs, vscID)
return true
})
}

if err != nil {
return err
}

// clean up indexes
for _, id := range vscIDs {
k.DeleteUnbondingOpIndex(ctx, chainID, id)
}

return nil
}

Expand Down

0 comments on commit 12ac66c

Please sign in to comment.